-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Restrict filename parameter of compile()
(and other stdlib functions wrapping compile()
) to take bytes, not Buffer.
#14847
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Restrict filename parameter of compile()
(and other stdlib functions wrapping compile()
) to take bytes, not Buffer.
#14847
Conversation
…sses to only take `bytes` instead of `Buffer` as path. These also route to `compile()`.
This comment has been minimized.
This comment has been minimized.
compile()
(and other stdlib functions that use compile()
) to only take bytes, not Buffer.compile()
(and other stdlib functions wrapping compile()
) to take bytes, not Buffer.
…tes instead of Buffer as path.
This comment has been minimized.
This comment has been minimized.
I'd say we shouldn't bother with the different version_info branches. Just use the more restrictive option. The possibility of someone using a non-bytes readable buffer and not wanting to fix it for Python < 3.12 are low, and I don't have much sympathy. |
Fair enough. |
…lib_external.pyi).
Diff from mypy_primer, showing the effect of this PR on open source code: altair (https://github.com/vega/altair)
+ altair/utils/execeval.py:81: error: Argument "filename" to "compile" has incompatible type "str | Buffer | PathLike[Any]"; expected "str | bytes | PathLike[Any]" [arg-type]
+ altair/utils/execeval.py:87: error: Argument "filename" to "compile" has incompatible type "str | Buffer | PathLike[Any]"; expected "str | bytes | PathLike[Any]" [arg-type]
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Fixes #13881.
Maybe
warnings.deprecated()
could be used here to deprecate non-bytes buffers as arguments, but I wanted to be initially conservative in the number of overloads this adds.