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

fix: incorrect returned type of access descriptors on unions of types #16604

Merged

Conversation

md384
Copy link
Contributor

@md384 md384 commented Dec 1, 2023

Fixes #16603

This change maps over union types when determining the types of access descriptors. Previously, the because this conditional would fall through to the else case because instance type was not a singular TypeType (it was a Union), so we'd end up with an instance value being passed to __get__ instead of None.

This comment has been minimized.

@md384 md384 force-pushed the m.descriptor-access-for-union-of-types branch from c2a55af to 3ab43bc Compare December 18, 2023 19:15
Copy link
Contributor

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

pandas (https://github.com/pandas-dev/pandas)
+ pandas/core/apply.py:265: error: Unused "type: ignore" comment  [unused-ignore]
+ pandas/core/apply.py:265: error: No overload variant of "__get__" of "AxisProperty" matches argument types "GroupBy[Any]", "type[GroupBy[Any]]"  [call-overload]
+ pandas/core/apply.py:265: note: Error code "call-overload" not covered by "type: ignore" comment
+ pandas/core/apply.py:265: note: Possible overload variants:
+ pandas/core/apply.py:265: note:     def __get__(self, obj: DataFrame | Series, type: Any) -> Index
+ pandas/core/apply.py:265: note:     def __get__(self, obj: None, type: Any) -> AxisProperty
+ pandas/core/apply.py:265: error: No overload variant of "__get__" of "AxisProperty" matches argument types "SeriesGroupBy", "type[SeriesGroupBy]"  [call-overload]
+ pandas/core/apply.py:265: error: No overload variant of "__get__" of "AxisProperty" matches argument types "DataFrameGroupBy", "type[DataFrameGroupBy]"  [call-overload]
+ pandas/core/apply.py:265: error: No overload variant of "__get__" of "AxisProperty" matches argument types "BaseWindow", "type[BaseWindow]"  [call-overload]
+ pandas/core/apply.py:265: error: No overload variant of "__get__" of "AxisProperty" matches argument types "Resampler", "type[Resampler]"  [call-overload]

@md384
Copy link
Contributor Author

md384 commented Mar 22, 2024

@JukkaL could you or another maintainer take a look at this PR?

It's (hopefully) a fairly simple fix to a fairly common sqlalchemy typing complaint.

Thanks

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

@hauntsaninja hauntsaninja merged commit 0570f71 into python:master Apr 16, 2024
18 checks passed
svalentin pushed a commit that referenced this pull request Apr 16, 2024
…#16604)

Fixes #16603

This change maps over union types when determining the types of access
descriptors. Previously, the because [this
conditional](https://github.com/md384/mypy/blob/c2a55afcef32ecb11a4c76c4c79539f6ba36d55c/mypy/checkmember.py#L697-L701)
would fall through to the `else` case because instance type was not a
singular `TypeType` (it was a Union), so we'd end up with an instance
value being passed to `__get__` instead of `None`.
@aleksanb
Copy link
Contributor

This PR fixed and revealed a new error with typing django models w/django-stubs.
Unsure whether to post here or on django-stubs so i'll start here on this PR:

Both Invoice and GroupInvoice here are django Models being pointed to from a third model, SalesDocument.

class SalesDocument(models.Model):
    invoice = models.OneToOneField(Invoice, on_delete=models.PROTECT, related_name='sales_document')
    group_invoice = models.OneToOneField(GroupInvoice, on_delete=models.PROTECT, related_name='sales_document')

But the following code errors out:

def foo(invoice: Invoice | GroupInvoice) -> bytes:
    invoice.sales_document  # type: ignore[call-overload]

After this patch, mypy is able to reveal the type of invoice.sales_document correctly as SalesDocument,
but errors out on the type of the overloaded sales_document method that django-stubs makes on both Invoice and GroupInvoice.

.../invoice.py:246: error: No overload variant of "__get__" of "ReverseOneToOneDescriptor" matches argument types "GroupInvoice", "type[GroupInvoice]"  [call-overload]
.../invoice.py:246: note: Possible overload variants:
.../invoice.py:246: note:     def __get__(self, instance: None, cls: Any = ...) -> ReverseOneToOneDescriptor[Invoice, SalesDocument]
.../invoice.py:246: note:     def __get__(self, instance: Invoice, cls: Any = ...) -> SalesDocument

@md384
Copy link
Contributor Author

md384 commented Apr 25, 2024

@aleksanb I suspect this is a django-stubs bug in the plugin code. Please file in that repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect returned type of access descriptors on unions of types
3 participants