Skip to content

Change type read-only attributes from @property to Final - #16554

Open
crypt0lith wants to merge 1 commit into
python:mainfrom
crypt0lith:fix/type-replace-property-final
Open

crypt0lith wants to merge 1 commit into
python:mainfrom
crypt0lith:fix/type-replace-property-final

Conversation

@crypt0lith

Copy link
Copy Markdown
Contributor

Follow-up to #16549

@github-actions

Copy link
Copy Markdown
Contributor

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

scipy-stubs (https://github.com/scipy/scipy-stubs)
+ tests/misc/test_submodules_empty.pyi:3: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports

@JelleZijlstra

Copy link
Copy Markdown
Member

Final also means they can't be overridden in subclasses. Is that accurate for these attributes?

@crypt0lith

Copy link
Copy Markdown
Contributor Author

Python does not prohibit them from being overridden, but doing so causes Python to disagree with the C implementation:

>>> class Meta(type):
...     __base__ = str
...     __basicsize__ = \
...     __dictoffset__ = \
...     __flags__ = \
...     __itemsize__ = \
...     __weakrefoffset__ = 42
...     __text_signature__ = "hello"
...     
>>> class Foo(metaclass=Meta):
...     pass
...     
>>> print(
...     *(
...         (getattr(Foo, attr), type.__dict__[attr].__get__(Foo, Meta))
...         for attr in [
...             "__base__",
...             "__basicsize__",
...             "__dictoffset__",
...             "__flags__",
...             "__itemsize__",
...             "__weakrefoffset__",
...             "__text_signature__",
...         ]
...     ),
...     sep="\n",
... )
(<class 'str'>, <class 'object'>)
(42, 16)
(42, -1)
(42, 22044)
(42, 0)
(42, -32)
('hello', None)

So Final here is explicitly declaring that those attributes should not be overridden.

inspect.isabstract accesses __flags__, so that is one example where allowing an override would be unsafe.

This branch has not been deployed

No deployments
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