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: Add DatetimeIndex.intersection() method and test #745

Closed
wants to merge 4 commits into from

Conversation

teacherc
Copy link

Fix DatetimeIndex.intersection() return type

The return type of DatetimeIndex.intersection() was incorrectly
specified.This updates the method signature to indicate the correct return
type of DatetimeIndex.

Test added as well.

Fixes #744

  • Closes #xxxx (Replace xxxx with the Github issue number)
  • Tests added: Please use assert_type() to assert the type of any return value

pandas-stubs/core/indexes/datetimes.pyi Outdated Show resolved Hide resolved
tests/test_indexes.py Outdated Show resolved Hide resolved
@teacherc teacherc requested a review from Dr-Irv July 17, 2023 15:45
@@ -97,6 +97,7 @@ class DatetimeIndex( # type: ignore[misc]
def tzinfo(self) -> tzinfo | None: ...
@property
def dtype(self) -> np.dtype | DatetimeTZDtype: ...
def intersection(self, other: DatetimeIndex | list) -> DatetimeIndex: ...
Copy link
Collaborator

Choose a reason for hiding this comment

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

list is too broad. I think Sequence[DatetimeLike] will work where you import DatetimeLike from pandas._typing


result = idx1.intersection(idx2)

check(assert_type(result, pd.DatetimeIndex), pd.DatetimeIndex)
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you include the list[DatetimeLike] as I suggested, then you should include tests that do intersection() with lists of those 3 different types. E.g., idx1.intersection([pd.Timestamp(x) for x in idx2]) and something similar for dt.datetime and np.datetime64

@Dr-Irv
Copy link
Collaborator

Dr-Irv commented Jul 17, 2023

Also, you are failing in CI because of one (or both) of two reasons:

  1. You are missing the sort: bool = False in the declaration. (You definitely have to do this)
  2. mypy may be being picky about restricting the types. So first try fixing (1), and if mypy still complains, then add a # type: ignore on the line declaring intersection()

@teacherc
Copy link
Author

@Dr-Irv Thank you for the feedback! I will try to have a better version of this up by the weekend or early next week.

@twoertwein
Copy link
Member

I believe this has been addressed as part of #760 . Feel free to re-open this PR if #760 didn't cover all aspects.

@twoertwein twoertwein closed this Aug 16, 2023
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.

Index.intersection does not return correct sub type
3 participants