Skip to content

Conversation

cmp0xff
Copy link
Contributor

@cmp0xff cmp0xff commented Sep 2, 2025

It belongs to the series of changes suggested by #1347, although not in the sense of a Pandas Series.

  • Tests added: Please use assert_type() to assert the type of any return value

Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

Looks pretty good

@cmp0xff cmp0xff requested a review from Dr-Irv September 2, 2025 22:37
Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

The only thing I'm worried about here is that we don't necessarily have a complete set of tests for subtracting timestamps, timedeltas, series, indexes, lists of such from DatetimeIndex and TimedeltaIndex, or from a plain Index where we detect an error. There are some that exist and are scattered around the tests.

Maybe we do a separate PR for that? Or handle here?

@cmp0xff
Copy link
Contributor Author

cmp0xff commented Sep 3, 2025

I understand that a set of systematic tests is desirable whenever the corresponding code is modified. In this case, however, I would personally prefer to develop these tests in a separate pull request or issue.

It has been quite exhausting to work out the timestamp and timedelta subtractions; the challenge may trace back to the fact that the subtraction here is not closed in the algebraic sense.

If needed, I can also revert the changes in DatetimeIndex and TimedeltaIndex, so that they are unmodified in this PR.

@cmp0xff cmp0xff requested a review from Dr-Irv September 3, 2025 21:26
Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

thanks @cmp0xff

@Dr-Irv Dr-Irv merged commit 928df7b into pandas-dev:main Sep 3, 2025
13 checks passed
@Dr-Irv
Copy link
Collaborator

Dr-Irv commented Sep 3, 2025

I understand that a set of systematic tests is desirable whenever the corresponding code is modified. In this case, however, I would personally prefer to develop these tests in a separate pull request or issue.

That's fine.

@cmp0xff cmp0xff deleted the feature/cmp0xff/index-sub branch September 4, 2025 16:17
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.

2 participants