Skip to content

Rename variable, OSError.filename will have union type (typeshed #1852) #4541

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

Closed
wants to merge 1 commit into from

Conversation

intgr
Copy link
Contributor

@intgr intgr commented Feb 4, 2018

See python/typeshed#1852, I want to change the type of OSError.filename to Union[str, bytes, None], but since the path variable is inferred to have str type, this causes a Mypy error.

On the other hand, the path = error.filename assignment seems of questionable value anyway, since path already contained the path. But I may be missing something so I erred on the side of caution of not (potentially) changing the behavior.

…on#1852)

The 'path' variable is inferred to have str type, but OSError.filename
really is Union[str, bytes, None], which causes a Mypy error.
@gvanrossum
Copy link
Member

I don't think this is the right thing to do. Unions are terrible from a gradual typing perspective. Your best bet may be to change the attribute in typeshed to Any, and then you won't need this PR.

intgr added a commit to intgr/typeshed that referenced this pull request Feb 5, 2018
ambv pushed a commit to python/typeshed that referenced this pull request Feb 14, 2018
Starting with Python 3.3, IOError, EnvironmentError and WindowsError are
aliases for OSError, which has all the attributes.

Reference:
* https://docs.python.org/3/library/exceptions.html#OSError
* https://www.python.org/dev/peps/pep-3151/

* OSError: Drop Python <3.3 compatibility
* Use Any instead of Union for filename/filename2 type, per GvR comment
See: python/mypy#4541
@intgr
Copy link
Contributor Author

intgr commented Feb 14, 2018

Merged in Typeshed; this is now unnecessary, closing. python/typeshed#1852

@intgr intgr closed this Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants