Compileall rx argument fix #5172
Conversation
|
I get the pytype error you saw if I import Protocol from typing_extensions. That appears to be due to an oversight in the pytype stub parsing code. (We should also be checking for "typing_extensions.Protocol" there.) This'll be fixed in next week's pytype release. In the meantime, try either writing |
|
Thanks Rebecca. |
Inheriting from Protocol[T] is a shorthand for inheriting from both Protocol and Generic[T]. We were handling this case properly for typing.Protocol but not for typing_extensions.Protocol. Context: python/typeshed#5172 PiperOrigin-RevId: 366414389
stdlib/compileall.pyi
Outdated
| if sys.version_info >= (3, 9): | ||
| def compile_dir( | ||
| dir: AnyPath, | ||
| maxlevels: Optional[int] = ..., | ||
| ddir: Optional[AnyPath] = ..., | ||
| force: bool = ..., | ||
| rx: Optional[Pattern[Any]] = ..., | ||
| rx: Optional[_SupportsSearch[AnyStr]] = ..., |
There was a problem hiding this comment.
The AnyStr doesn't make sense to me here; how is the typevar going to get constrained? I'm guessing it depends on whether dir is a bytes or a str path, so perhaps that argument should also have a TypeVar in it.
(Note that AnyPath and AnyStr are very different things: the first is a union and the second a TypeVar.)
There was a problem hiding this comment.
OK, so compile_dir runs compile_file on every file it can using listdir - which means file will be bytes or str, so rx there can accept str and bytes. which is why I used AnyStr, a type that can be either or.
compile_file however gets AnyPath as file name and can pass that along to rx as is if quiet is 2. Meaning that the rx in compile_file might need to handle the same type as fullname. Which means I might need to fix that? Not sure what to do there.
Either way, I'm not following you. What did you have in mind?
There was a problem hiding this comment.
Well, the place I started with is that having a single TypeVar in a signature tends to be a mistake, because the point of a typevar is to use the same type in multiple places. See the discussion in https://mail.python.org/archives/list/typing-sig@python.org/message/NRFNHGXHXPGBR6FP3TIOZZ6VS4XJZX6K/ for some context.
But I just tried it out and at least in 3.7, we don't need all these Unions: compile_dir actually only works on str paths, not bytes paths. In compile_file, it calls importlib.util.cache_from_source, and that only accepts str, not bytes. So instead, let's just change all the parameters dealing with paths to accept only strs.
There was a problem hiding this comment.
I've changed things to StrPath since it does work with PathLike[str].
|
Thanks @hatal175! @hauntsaninja would you mind double-checking me about bytes paths not working at all with |
|
I did. Tried bytes, str and their Path equivalents.
…On Sun, Apr 11, 2021, 05:22 Jelle Zijlstra ***@***.***> wrote:
Thanks @hatal175 <https://github.com/hatal175>!
@hauntsaninja <https://github.com/hauntsaninja> would you mind
double-checking me about bytes paths not working at all with compileall?
I see we discussed this in #3956
<#3956> before, and the
documentation actually claims bytes paths are supported.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5172 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACYQXXKHF3YJRMCJNF376X3TIEBXZANCNFSM42IAN5IQ>
.
|
There was a problem hiding this comment.
Thanks, yeah, the compileall documentation is full of falsehood unfortunately. fullname has to be StrPath because of importlib, ddir has to be StrPath because os.path.join(ddir, os.path.basename(os.fspath(fullname))) (or None), similar for prependdir.
However, limit_sl_dest can still be bytes, and is documented as that being the case, so we should change that one back to AnyPath.
Separately, it looks like compile_dir's dir can be None under some expressible via overload circumstances.
|
Could you give examples of both? And I don't quite see what code path will make them work. |
Fixing #3480.
I've added this to all versions since the code seems to use only the search method.
Now, I didn't really want to add SupportsSearch to _typeshed but for some reason, pytype really didn't like it when I put it in the same file.
See here: https://github.com/hatal175/typeshed/runs/2249491861?check_suite_focus=true
I suspect it might have something to with google/pytype#296 or that Protocol isn't subscriptable in pytype stubs but I can't be sure.
If you have other suggestions, I'll be happy to use them.