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

TYP: cache_readonly #43252

Closed
wants to merge 8 commits into from
Closed

TYP: cache_readonly #43252

wants to merge 8 commits into from

Conversation

twoertwein
Copy link
Member

@twoertwein twoertwein commented Aug 27, 2021

I'm not sure how to test this on the CI - pyright is happy on my machine:

import pandas as pd

index: pd.IntervalIndex = pd.IntervalIndex.from_arrays([1], [2])
reveal_type(index)
reveal_type(index.right)
info: Type of "index" is "IntervalIndex"
info: Type of "index.right" is "Index"


class cache_readonly(Protocol[_F, _G]):
def __init__(self, func: Callable[[_F], _G]) -> None: ...
def __get__(self, obj: _F, typ) -> _G: ...
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know what the variable typ is.

The return value of __get__ can also be self (is that intended?):

Copy link
Member

Choose a reason for hiding this comment

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

The return value of get can also be self (is that intended?):

Yes, this happens when you lookup cls.foo on the class rather than on an instance

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I added an overload for this.

The current type stubs do not handle this case

except (AttributeError):

If the only solution to cover this case is to return _G | cache_readonly[_F, _G], I am inclined to not cover this case as it would make the annotations less useful (even though it would be correct): index.right would then be a union of two types.

@twoertwein twoertwein added the Typing type annotations, mypy/pyright type checking label Aug 28, 2021
@jbrockmendel
Copy link
Member

If the CI failures prove hard to resolve, could we cheat and write in the .pyi something like class cache_readonly(property): pass?

@twoertwein
Copy link
Member Author

If the CI failures prove hard to resolve, could we cheat and write in the .pyi something like class cache_readonly(property): pass?

I must admit, I didn't notice the many mypy failures. I will put the PR into draft mode and locally debug it until I get it working. I will ping you when it is ready :)

@twoertwein twoertwein marked this pull request as draft August 29, 2021 18:29
@twoertwein
Copy link
Member Author

twoertwein commented Aug 29, 2021

If the CI failures prove hard to resolve, could we cheat and write in the .pyi something like class cache_readonly(property): pass?

I mistakenly used Protocol instead of Generic: roughly half of the errors are gone now. There are still many legitimate errors left.

Some remaining [arg-type] errors on unions and [override] errors are probably due to a mypy issue (opened python/mypy#11037).

@twoertwein
Copy link
Member Author

see #41588 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Typing type annotations, mypy/pyright type checking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TYP: IntervalIndex.right/left might be infered as function
2 participants