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 issue for pandera allowing generic Series to work #492

Merged
merged 3 commits into from
Jan 3, 2023

Conversation

Dr-Irv
Copy link
Collaborator

@Dr-Irv Dr-Irv commented Jan 2, 2023

Fix here is to expand the Series[IntervalT].__new__() into each individual part. Seems like a double TypeVar causes issues for mypy. Had to do a similar fix for Interval.__contains__()

Either @twoertwein or @bashtage can approve or merge (both not required)

index: Axes | None = ...,
dtype=...,
name: Hashable | None = ...,
copy: bool = ...,
fastpath: bool = ...,
) -> Series[IntervalT]: ...
) -> Series[Interval[int]]: ...
@overload
Copy link
Member

Choose a reason for hiding this comment

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

might be nice to add a TODO referencing the mypy issue

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

might be nice to add a TODO referencing the mypy issue

I didn't research whether there exists a mypy issue, or how to create a small sample to replicate.

I could add a comment indicating that we prefer the Series[IntervalT] notation, but that it doesn't seem to work and reference the original pandas-stubs issue. Let me know if I should do that.

Copy link
Member

Choose a reason for hiding this comment

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

Would something like the following work?

T = TypeVar("T", float, int, Timestamp, Timedelta)

        data: IntervalIndex[Interval[T]],
        ....
    ) -> Series[Interval[T]]: ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can't see how this would work, because we have:

class IntervalIndex(IntervalMixin, ExtensionIndex, Generic[IntervalT]):

Since IntervalIndex is generic, and Interval is generic, you can't make IntervalIndex depend on the "inner" generic type.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, sorry for the trouble.

@twoertwein twoertwein merged commit 115dd5c into pandas-dev:main Jan 3, 2023
@twoertwein
Copy link
Member

Thanks @Dr-Irv !

@Dr-Irv Dr-Irv deleted the issue471 branch February 4, 2023 17:07
twoertwein pushed a commit to twoertwein/pandas-stubs that referenced this pull request Apr 1, 2023
* split Series[IntervalT] into separate __new__

* add test for generic series

* Update reference to GH issue
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.

Incorrect "Type application has too few types" when constructing generic Series type
2 participants