8000 Update `ModuleType` to have `__path__` optionally by ewerybody · Pull Request #6200 · python/typeshed · GitHub
[go: up one dir, main page]
Skip to content

Update ModuleType to have __path__ optionally #6200

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Oct 25, 2021
Merged

Update ModuleType to have __path__ optionally #6200

merged 4 commits into from
Oct 25, 2021

Conversation

ewerybody
Copy link
Contributor

Hello typeshed crew!
Analog to #6186 I'd like to add __path__ to ModuleType.

Custom packages always have this attribute and currently getting __path__ from these via sys.modules like for instance:

sys.modules['mymodule_pack'].__path__

results in typing complaints like

Cannot access member "__path__" for type "ModuleType"
  Member "__path__" is unknown

Since __file__ is already implemented. __path__ would also be good. Thanks.
ëRiC

@github-actions

This comment has been minimized.

Sign in to view
hauntsaninja
hauntsaninja reviewed Oct 25, 2021
View reviewed changes
Copy link
Collaborator
@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this should be a list. Also, under what circumstances is this ever None?

Actually looking closer, sometimes this can be at least a _NamespacePath (which is a MutableSequence).

@ewerybody
Copy link
Contributor Author

... under what circumstances is this ever None?

Yeah arguable. In the other PR they were discussing about this because it may not exist at all which cannot be expressed. Or can it?

Actually looking closer, sometimes this can be at least a _NamespacePath (which is a MutableSequence).

Oh ok. Like option to list[str] | MutableSequence?
I coulnd't find _NamespacePath tho.

@github-actions

This comment has been minimized.

Sign in to view
@ewerybody
Copy link
Contributor Author

Thanks for reviewing! 🙏

Akuli
Akuli reviewed Oct 25, 2021
View reviewed changes
@Akuli
Copy link
Collaborator
Akuli commented Oct 25, 2021 •
edited
Loading

We don't have a way to express "__path__ may be missing", but I don't think it's a problem in practice:

  • __file__ is missing in obscure corner cases. It would be good if the type checker warned about not checking that.
  • __path__ is missing usually (specifically, for imported .py files). If you know anything at all about __path__, you are probably aware of this, or you will find out by doing some very basic testing, without checking corner cases.

ewerybody and others added 2 commits October 25, 2021 22:05
@ewerybody @Akuli
Update stdlib/types.pyi
96532a5 
`ModuleType.__path__` to be `MutableSequences[str]` covering `list` and `_NamespacePath`

Co-authored-by: Akuli <akuviljanen17@gmail.com>
@ewerybody
importing MutableSequence
9fad548
@github-actions

This comment has been minimized.

Sign in to view
1 similar comment
@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

core (https://github.com/home-assistant/core.git)
+ homeassistant/loader.py:298: error: unused "type: ignore" comment

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

Akuli
Akuli approved these changes Oct 25, 2021
View reviewed changes
Copy link
Collaborator
@Akuli Akuli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Akuli Akuli merged commit fd35084 into python:master Oct 25, 2021
@ewerybody
Copy link
Contributor Author

a pleasure 🙇‍♀️

@cdce8p cdce8p mentioned this pull request Dec 21, 2021
Open
@bzoracler bzoracler mentioned this pull request Jun 16, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hauntsaninja hauntsaninja hauntsaninja left review comments

@Akuli Akuli Akuli approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ewerybody @Akuli @hauntsaninja
0