FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Mark object.__module__ as ClassVar by hauntsaninja · Pull Request #8787 · python/typeshed · GitHub

Repository navigation

Mark object.__module__ as ClassVar - #8787

Closed
hauntsaninja wants to merge 1 commit into
python:mainfrom
hauntsaninja:module-classvar
Closed

hauntsaninja wants to merge 1 commit into
python:mainfrom
hauntsaninja:module-classvar

Conversation

hauntsaninja commented Sep 23, 2022 •
edited
Loading

Copy link
Copy Markdown
Collaborator

The reported use case for this isn't particularly compelling, so if this causes too many errors I will close.

Copy link
Copy Markdown
Member

I kinda think object.__module__ shouldn't actually exist:

>>> object().__module__
Traceback (most recent call last):
  File "<string>", line 1, in <module>
AttributeError: 'object' object has no attribute '__module__'

Copy link
Copy Markdown
Contributor

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

streamlit (https://github.com/streamlit/streamlit)
+ lib/tests/streamlit/runtime/caching/hashing_test.py:29: note: ... from here:

SinbadCogs (https://github.com/mikeshardmind/SinbadCogs)
+ devtools/core.py:193: error: Cannot assign to class variable "__module__" via instance

yarl (https://github.com/aio-libs/yarl)
+ yarl/_url.py:19: error: Cannot assign to class variable "__module__" via instance  [misc]

werkzeug (https://github.com/pallets/werkzeug)
+ src/werkzeug/utils.py:89: error: Cannot assign to class variable "__module__" via instance  [misc]

hauntsaninja commented Sep 23, 2022 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Looking at them, I'm fine with taking these primer hits, but I'm a little worried about all the stubs that have this.

I'm happy to have object.__module__ exist, it's in line with several things we do. There's a small chance removing it wouldn't be the worst, though, since we still have __module__ on type.

Copy link
Copy Markdown
Member

I'm happy to have object.__module__ exist, it's in line with several things we do.

I know, but I'm not sure I'm super jazzed about those things either ¯\_(ツ)_/¯

AlexWaygood added a commit that referenced this pull request Sep 24, 2022
An alternative approach to #8787

Copy link
Copy Markdown
Member

There's a small chance removing it wouldn't be the worst, though, since we still have __module__ on type.

#8789 went down like a bag of sick, so if we want to fix this, this does indeed look like the better solution, at least for now :)

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL