Skip to content

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

@hauntsaninja hauntsaninja commented Sep 23, 2022 •

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.

@AlexWaygood

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__'

@github-actions

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

hauntsaninja commented Sep 23, 2022 •

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.

@AlexWaygood

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
@AlexWaygood

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 :)

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