Skip to content
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

Missing attributes for MappingView and subclasses in stdlib/collections_abc_.pyi #7576

Closed
laundmo opened this issue Apr 1, 2022 · 8 comments

Comments

@laundmo
Copy link

laundmo commented Apr 1, 2022

Due to stdlib/_collections_abc.pyi importing most directly from stdlib/typing.pyi, MappingView and its subclasses are missing the self._mapping attribute. This is an issue if inheriting from them for implementing custom views. This causes pyright and mypy (with --check-untyped-defs) to fail.

Minimal Example:

import collections.abc

class MyView(collections.abc.KeysView):
    def __iter__(self):
        for key in self._mapping:
            yield key + 1

mypy --check-untyped-defs test.py

test.py:5: error: "MyView" has no attribute "_mapping"

pyright test.py

test.py
  test.py:5:25 - error: Cannot access member "_mapping" for type "MyView"
    Member "_mapping" is unknown (reportGeneralTypeIssues)
1 error, 0 warnings, 0 informations

I'm open to trying to PR this myself, though I have little experience writing stub files.

@AlexWaygood
Copy link
Member

AlexWaygood commented Apr 1, 2022

Hmm, this attribute is marked as private, and doesn't appear to be documented anywhere. Can I ask what your use case is for accessing it? 🙂

We're generally happy to add undocumented undocumented/private attributes to stubs if there's a valid use case, but we need to be more cautious when making changes to typing.pyi, as these can have big impacts on the rest of the typing ecosystem.

@laundmo
Copy link
Author

laundmo commented Apr 1, 2022

@AlexWaygood I would propose only adding them to stdlib/_collections_abc.pyi and not to typing.pyi, as they seem to be specific to the collections.abc implementation of MappingView, ValuesView etc.

In fact, I would be against adding them to typing.pyi as the attribute is not needed when implementing a completely custom MappingView that doesn't inherit from collections.abc.

As for the use case, in my case I wanted to create a custom mapping class that would convert the keys automatically, for which i need custom views to apply the conversions in case someone uses .items(), .keys() etc. on my custom class.

@AlexWaygood
Copy link
Member

If you're proposing decoupling the types in collections.abc from the types in typing, that would be a significant undertaking 🙂 (see #6257 for previous discussion and attempts to do just that). I'd advise not trying to do that.

@Akuli
Copy link
Collaborator

Akuli commented Apr 1, 2022

This is similar to #7153.

@AlexWaygood
Copy link
Member

This is similar to #7153.

Yes, though #7153 was regarding a method with an extensive docstring, whereas this attribute is entirely undocumented from what I can see.

@laundmo
Copy link
Author

laundmo commented Apr 1, 2022

This is similar to #7153.

Yes, though #7153 was regarding a method with an extensive docstring, whereas this attribute is entirely undocumented from what I can see.

entirely undocumented but also required if you want any tangible benefit from using collections.abc.MappingView and descendants. its really no big deal to work around, but since its even implemented in slots in the abc i do think you're meant to use it.

@Akuli
Copy link
Collaborator

Akuli commented Apr 1, 2022

Feel free to make a PR that adds _mapping :)

@AlexWaygood
Copy link
Member

Closing due to lack of response from OP on the PR: #7583 (comment)

Feel free to open a new PR with the requested changes, if you're still interested in this issue, @laundmo :)

@AlexWaygood AlexWaygood closed this as not planned Won't fix, can't repro, duplicate, stale Jul 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants