BUG: fix PeriodIndex.difference producing incorrect results #60992
Conversation
pandas/core/indexes/period.py
Outdated
| @@ -534,6 +534,22 @@ def shift(self, periods: int = 1, freq=None) -> Self: | |||
| ) | |||
| return self + periods | |||
|
|
|||
| def _convert_can_do_setop(self, other): | |||
| try: | |||
| if isinstance(other, Index): | |||
There was a problem hiding this comment.
A lot of this can be outside a try/except
There was a problem hiding this comment.
@jbrockmendel Thank you for the review, but before submitting the revision, I would like to confirm with you whether you think this modification direction is correct.
There was a problem hiding this comment.
@jbrockmendel Thank you for your previous feedback on my PR . I have addressed your suggestions and made the improvements accordingly. I would love to get your thoughts on the updated changes. Thanks!
| @@ -691,6 +691,7 @@ Indexing | |||
| - Bug in :meth:`DataFrame.__getitem__` returning modified columns when called with ``slice`` in Python 3.12 (:issue:`57500`) | |||
| - Bug in :meth:`DataFrame.from_records` throwing a ``ValueError`` when passed an empty list in ``index`` (:issue:`58594`) | |||
| - Bug in :meth:`MultiIndex.insert` when a new value inserted to a datetime-like level gets cast to ``NaT`` and fails indexing (:issue:`60388`) | |||
| - Bug in :meth:`PeriodIndex.difference` producing incorrect results when operating between :class:`PeriodIndex` and :class:`Index` objects (:issue:`58971`) | |||
There was a problem hiding this comment.
are the affected Index objects object-dtype?
| index2 = pd.Index(["2022-02", "2022-03"]) | ||
|
|
||
| result1 = index1.difference(index2) | ||
| expected1 = PeriodIndex(["2022-01", "2022-04", "2022-05"], freq="M") |
There was a problem hiding this comment.
i think this is wrong and the result in main is correct.
|
|
||
| # Convert non-PeriodIndex to PeriodIndex | ||
| try: | ||
| other = PeriodIndex(other, freq=self.freq) |
There was a problem hiding this comment.
i dont think we should be doing this casting implicitly. the user should do it explicitly if they want it.
|
This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this. |
|
Thanks for the pull request, but it appears to have gone stale. If interested in continuing, please merge in the main branch, address any review comments and/or failing tests, and we can reopen. |
Index.differenceis returning too many values #58971doc/source/whatsnew/v3.0.0.rstfile if fixing a bug or adding a new feature.