-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Add __path__ to package __init__ #9454
Conversation
I wasn't thrilled about the 200+ failed tests, so I added list to lib-stub's builtins. I guess I'm about to find out if that's acceptable :-) |
@@ -12,14 +12,6 @@ import m | |||
m.x # E: "object" has no attribute "x" | |||
[file m.py] | |||
|
|||
[case testListMissingFromStubs] |
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.
Why did you delete this test? The behavior it's testing (error messages tailored to bad fixtures) seems useful.
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.
The quickest way to resolve the many test failures was to add list to the builtin stubs (I expect this might be controversial), which of course broke this test.
We have tests for tailored error messages for several other specific fixtures, so I figured this wouldn't be the worst test to remove, especially since I anticipate pushback on adding list to builtin stubs.
This gives us better errors for fixtures stuff and helps with Python 2
@JelleZijlstra what else needs to be done here? This fixes the first mypy bug many users will run into. |
I'd prefer for @JukkaL to review it. |
The build seems to be stuck. If you can make this up to date with master so that there's a passing build, I'm happy to review this. |
Unstuck Travis, and all is green! |
@JukkaL @hauntsaninja looks like we were really close on this! Any chance we revisit and get it merged? 🤞 |
@JukkaL @hauntsaninja bump for approval :) |
This comment has been minimized.
This comment has been minimized.
Yes, technically it returns whatever its first argument is if it isn't a list. Doing this because python/mypy#9454 (comment)
This comment has been minimized.
This comment has been minimized.
Python 3.5 CI failed with some SSL error (as seems to happen half the time now). I restarted it. |
Yes, technically it returns whatever its first argument is if it isn't a list. Doing this because python/mypy#9454 (comment)
Diff from mypy_primer, showing the effect of this PR on open source code: porcupine (https://github.com/Akuli/porcupine.git)
+ porcupine/images/__init__.py:9: error: Name "__path__" already defined (possibly by an import) [no-redef]
+ porcupine/plugins/__init__.py:13: error: Name "__path__" already defined (possibly by an import) [no-redef]
poetry (https://github.com/python-poetry/poetry.git)
- poetry/__init__.py:4: error: Cannot determine type of "__path__"
+ poetry/mixology/failure.py:144: error: Item "RootCause" of "Union[RootCause, NoVersionsCause, DependencyCause, ConflictCause, PythonCause, PlatformCause, PackageNotFoundCause]" has no attribute "conflict"
edgedb (https://github.com/edgedb/edgedb.git)
+ edb/schema/std.py:41: error: unused "type: ignore" comment
+ edb/schema/std.py:42: error: unused "type: ignore" comment
+ edb/schema/std.py:43: error: unused "type: ignore" comment
+ edb/schema/std.py:44: error: unused "type: ignore" comment
|
Resolves #1422
I see there's a small graveyard of attempted fixes to this issue. Hopefully the same fate does not befall this PR.