Repository navigation
[ty] Avoid writable override contracts from object - #29373
Conversation
Memory usage reportMemory usage unchanged ✅ |
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 98.24%. The percentage of expected errors that received a diagnostic held steady at 98.24%. The number of fully passing files held steady at 134/146. |
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-property-type-override |
0 | 12 | 0 |
invalid-mutable-override |
0 | 11 | 0 |
| Total | 0 | 23 | 0 |
Raw diff (23 changes)
dd-trace-py (https://github.com/DataDog/dd-trace-py)
- ddtrace/contrib/internal/django/utils.py:453:9 error[invalid-property-type-override] Invalid override of attribute `__module__`: Read-only attribute overrides a writable attribute
egglog-python (https://github.com/egraphs-good/egglog-python)
- python/egglog/runtime.py:458:9 error[invalid-property-type-override] Invalid override of attribute `__doc__`: Read-only attribute overrides a writable attribute
- python/egglog/runtime.py:620:9 error[invalid-property-type-override] Invalid override of attribute `__doc__`: Read-only attribute overrides a writable attribute
psycopg (https://github.com/psycopg/psycopg)
- psycopg/psycopg/_wrappers.py:19:5 error[invalid-mutable-override] Invalid override of attribute `__module__`: Override does not accept writes of type `str`
- psycopg/psycopg/_wrappers.py:37:5 error[invalid-mutable-override] Invalid override of attribute `__module__`: Override does not accept writes of type `str`
- psycopg/psycopg/_wrappers.py:55:5 error[invalid-mutable-override] Invalid override of attribute `__module__`: Override does not accept writes of type `str`
- psycopg/psycopg/_wrappers.py:73:5 error[invalid-mutable-override] Invalid override of attribute `__module__`: Override does not accept writes of type `str`
- psycopg/psycopg/_wrappers.py:91:5 error[invalid-mutable-override] Invalid override of attribute `__module__`: Override does not accept writes of type `str`
- psycopg/psycopg/_wrappers.py:109:5 error[invalid-mutable-override] Invalid override of attribute `__module__`: Override does not accept writes of type `str`
- psycopg/psycopg/_wrappers.py:127:5 error[invalid-mutable-override] Invalid override of attribute `__module__`: Override does not accept writes of type `str`
scipy (https://github.com/scipy/scipy)
- scipy/special/_multiufuncs.py:60:9 error[invalid-property-type-override] Invalid override of attribute `__doc__`: Read-only attribute overrides a writable attribute
scipy-stubs (https://github.com/scipy/scipy-stubs)
- scipy-stubs/special/_multiufuncs.pyi:49:9 error[invalid-property-type-override] Invalid override of attribute `__doc__`: Read-only attribute overrides a writable attribute
sympy (https://github.com/sympy/sympy)
- sympy/multipledispatch/dispatcher.py:320:9 error[invalid-property-type-override] Invalid override of attribute `__doc__`: Read-only attribute overrides a writable attribute
- sympy/assumptions/assume.py:185:9 error[invalid-property-type-override] Invalid override of attribute `__doc__`: Read-only attribute overrides a writable attribute
- sympy/core/cache.py:186:13 error[invalid-property-type-override] Invalid override of attribute `__doc__`: Read-only attribute overrides a writable attribute
- sympy/core/cache.py:200:13 error[invalid-property-type-override] Invalid override of attribute `__doc__`: Read-only attribute overrides a writable attribute
- sympy/core/kind.py:344:9 error[invalid-property-type-override] Invalid override of attribute `__doc__`: Read-only attribute overrides a writable attribute
- sympy/core/operations.py:708:9 error[invalid-property-type-override] Invalid override of attribute `__doc__`: Read-only attribute overrides a writable attribute
- sympy/core/relational.py:1522:5 error[invalid-mutable-override] Invalid override of attribute `__doc__`: Override does not accept writes of type `str | None`
- sympy/core/relational.py:1539:5 error[invalid-mutable-override] Invalid override of attribute `__doc__`: Override does not accept writes of type `str | None`
- sympy/core/relational.py:1557:5 error[invalid-mutable-override] Invalid override of attribute `__doc__`: Override does not accept writes of type `str | None`
xarray (https://github.com/pydata/xarray)
- xarray/plot/accessor.py:39:5 error[invalid-mutable-override] Invalid override of attribute `__doc__`: Override does not accept writes of type `str | None`
zope.interface (https://github.com/zopefoundation/zope.interface)
- src/zope/interface/tests/test_interface.py:364:17 error[invalid-property-type-override] Invalid override of attribute `__module__`: Read-only attribute overrides a writable attribute
Ecosystem reportPR #29373 ecosystem summaryThe PR removes 23 false-positive override diagnostics across eight projects. Two patterns account for the changes: getter-only properties that expose class metadata, and metadata assignments in slotted class bodies. Neither pattern needs to preserve instance writes that Analyzed PR head: Diagnostic changes (2 sections)1. Getter-only metadata properties12 removed Report entries
Egglog, SciPy, scipy-stubs, and SymPy expose computed documentation through getter-only Reduced from SciPy’s class MultiUFunc:
@property
# Merge base: [invalid-property-type-override] Invalid override of attribute `__doc__`: Read-only attribute overrides a writable attribute
# PR: no diagnostic
def __doc__(self):
...DjangoViewProxy forwards a wrapped callable’s module through a Reduced from dd-trace-py’s class DjangoViewProxy:
@property
# Merge base: [invalid-property-type-override] Invalid override of attribute `__module__`: Read-only attribute overrides a writable attribute
# PR: no diagnostic
def __module__(self):
...2. Metadata assignments in slotted class bodies11 removed Report entries
Psycopg gives seven numeric wrapper classes a public module name through Reduced from Psycopg’s numeric wrappers: class NumericWrapper:
# Merge base: [invalid-mutable-override] Invalid override of attribute `__module__`: Override does not accept writes of type `str`
# PR: no diagnostic
__module__ = "psycopg.types.numeric"
__slots__ = ()Xarray’s plotting accessor copies a plotting function’s documentation, and three SymPy comparison classes copy another comparison class’s documentation. These class-body assignments are likewise valid without writable instance storage. The particular documentation value is not essential to the error. Reduced from Xarray’s plotting accessor: class PlotAccessor:
__slots__ = ()
# Merge base: [invalid-mutable-override] Invalid override of attribute `__doc__`: Override does not accept writes of type `str | None`
# PR: no diagnostic
__doc__ = NoneReproductionDetails
|
sharkdp
left a comment
There was a problem hiding this comment.
Thank you.
Should this maybe be fixed in typeshed? Could object.__module__ be defined as a read-only property returning str? Or would that imply something beyond "has a read type of str"?
| read, | ||
| write: if is_final || !is_class_var && literal.is_frozen_dataclass(db) == Some(true) { | ||
| // `object` provides no instance attribute storage, so its declarations impose | ||
| // no write contract on subclasses. |
There was a problem hiding this comment.
I looked at this PR before looking at the issue and had trouble understanding both the PR summary and this comment (it makes sense now with more context, but in isolation, I didn't understand). You wrote a nice explanation of the problem here. I think it would make for a better source comment here. Maybe also prefix it with "attributes on object are included here, because ..." to make it clear that the comment only refers to the first condition in the OR chain.
| `object` does not provide writable instance attributes. Its attribute declarations constrain read | ||
| types, but do not require subclasses to support instance writes. | ||
|
|
There was a problem hiding this comment.
I feel like this should maybe mention typeshed's object definition? Something like
typeshed declares attributes like
__module__: stron the classobject. Usually, this indicates the presence of a writable instance attribute...
It's tricky because it's reasonable for a type checker to also take that to mean "class access returns a property object". This kind of thing is the motivation for https://peps.python.org/pep-0767/ - if we get that, then I think it should definitely be applied to these attributes of |
|
On this PR branch, we now do not emit any errors on this: def f(x: object):
x.__module__ = "foo"
class Bar:
@property
def __module__(self) -> str:
return "foo"
f(Bar())If we say that the |
|
@AlexWaygood That's a good catch, but this particular unsound combination (allowing non-writable overrides of |
|
Ugh shoot I then also forgot to push my wording updates to address @sharkdp comments. Will put up a separate PR for that too 😆 |
Fix false
invalid-mutable-overridediagnostics when slotted classes assign metadata such as__module__. The attributes declared onobjectdo not establish an instance-write contract for subclasses, becauseobjectprovides no instance attribute storage. Read-type compatibility and write contracts explicitly declared by user-defined bases remain checked.Fixes astral-sh/ty#4725.
Test plan
Checked CPython 3.13 behavior: assignments to
__module__,__doc__,__annotations__,__dict__, and__str__onobject()fail. Also verified that__class__assignment fails forobject()but can succeed between compatible slotted user-defined classes.Ecosystem report
Analyzed head:
56847d246246b4381d4873a36495e0d91d89f469. The ecosystem shows 23 expected false-positive removals across eight projects: getter-only metadata properties and metadata assignments in slotted class bodies. All changes are intended, with no ecosystem concerns remaining. Full report.