Allow user to specify custom typeshed directory #1588
Conversation
ddfisher
force-pushed
the
PR/typeshed-flag
branch
from
a197a66 to
e26052f
Compare
| @@ -160,6 +162,8 @@ def parse_version(v): | |||
| help="an anti-pattern") | |||
| parser.add_argument('--stats', action='store_true', help="dump stats") | |||
| parser.add_argument('--inferstats', action='store_true', help="dump type inference stats") | |||
| parser.add_argument('--custom-typeshed', metavar='PATH', | |||
| help="use the custom typeshed module at PATH") | |||
There was a problem hiding this comment.
I wonder if the term 'typeshed module' is clear enough, as the term 'module' is overloaded? Maybe use 'typeshed repository'?
There was a problem hiding this comment.
What about "typeshed package"?
|
Looks good, other the note above about the help message. |
|
It's neither a package nor a module -- it's a directory though. Maybe now that you've redone the command line options you can rebase, rework, and resubmit this? I still think it's a great idea. (Maybe also add to the docs? There's an extensive chapter on the command line.) |
ddfisher
force-pushed
the
PR/typeshed-flag
branch
from
e26052f to
289f643
Compare
|
I think this is blocking on #1587 now. |
gvanrossum
changed the title
There was a problem hiding this comment.
It's a shame that this is blocked for that silly partial bug #1587... If you can work around that this can be committed after rebasing.
| options.custom_typeshed_dir.rstrip(os.sep)) | ||
| data_dir = custom_data_dir | ||
| else: | ||
| data_dir = default_data_dir(bin_dir) |
There was a problem hiding this comment.
Can you work around #1587 by setting custom_typeshed_name = None here?
There was a problem hiding this comment.
Yep, that fixes it!
ddfisher
force-pushed
the
PR/typeshed-flag
branch
from
289f643 to
5afdb9d
Compare
ddfisher
changed the title
|
Updated! Should be mergeable now. |
|
PS can you try not to squash all local commits? |
| @@ -326,6 +326,12 @@ Here are some more useful flags: | |||
| package, the behavior enabled by this flag is often more | |||
| convenient.) | |||
|
|
|||
| - ``--custom-typeshed-dir DIR`` specifies the directory where mypy looks for | |||
There was a problem hiding this comment.
Can you also update config_file.rst?
| options.custom_typeshed_dir.rstrip(os.sep)) | ||
| data_dir = custom_data_dir | ||
| else: | ||
| custom_typeshed_name = None |
There was a problem hiding this comment.
Or maybe 'typeshed' so you won't have to fill that in later? (Then perhaps drop the 'custom_' prefix.)
There was a problem hiding this comment.
Yeah, that's nicer. Changed.
There was a problem hiding this comment.
Actually, this got changed in a different way from the refactor to fix the other issues.
| @@ -266,7 +277,7 @@ def default_lib_path(data_dir: str, pyversion: Tuple[int, int]) -> List[str]: | |||
| # (Note that 3.1 and 3.0 aren't really supported, but we don't care.) | |||
| for v in versions + [str(pyversion[0]), '2and3']: | |||
| for lib_type in ['stdlib', 'third_party']: | |||
| stubdir = os.path.join(data_dir, 'typeshed', lib_type, v) | |||
| stubdir = os.path.join(data_dir, typeshed_name, lib_type, v) | |||
There was a problem hiding this comment.
It's somewhat perverse that the flag first gets separated into a data_dir and a typeshed_name and then they are joined again here. And don't the other uses of data_dir care that it's been overridden? (It's passed to Reports() which uses it to locate some style sheets.)
| """Return default standard library search paths.""" | ||
| # IDEA: Make this more portable. | ||
| path = [] # type: List[str] | ||
| typeshed_name = custom_typeshed_name or 'typeshed' | ||
|
|
||
| auto = os.path.join(data_dir, 'stubs-auto') |
There was a problem hiding this comment.
This block (three lines starting here) probably has to be skipped if a custom typeshed was given.
There was a problem hiding this comment.
What's going on here? What is stubs-auto? (The commit that introduces it doesn't explain at all...)
| @@ -188,6 +188,8 @@ def process_options(args: List[str], | |||
| help="dump type inference stats") | |||
| parser.add_argument('--custom-typing', metavar='MODULE', dest='custom_typing_module', | |||
| help="use a custom typing module") | |||
| parser.add_argument('--custom-typeshed-dir', metavar='DIR', | |||
| help="use the custom typeshed in DIR") | |||
There was a problem hiding this comment.
I think Jukka had suggested to change this to "custom typeshed package in DIR" ?
There was a problem hiding this comment.
Looking through the old comments, we noted that typeshed wasn't really a package -- that's why I left that out here.
|
@gvanrossum can you take another look at this? |
Current coverage is 83.48% (diff: 66.66%)@@ master #1588 diff @@
==========================================
Files 72 72
Lines 19012 19017 +5
Methods 0 0
Messages 0 0
Branches 3931 3932 +1
==========================================
+ Hits 15873 15876 +3
- Misses 2542 2543 +1
- Partials 597 598 +1
|
|
🎉 |
This avoids doing any sort of cleanup of
default_data_dirto be as safe of a change as possible.Triggers #1587 and therefore blocks on it.