Skip to content

Conversation

@mroeschke
Copy link
Member

Minor, but I think key is a scalar so all(x is NaT for x in exp[key]) wouldn't work since Periods are not iterable. Additionally, I think given the construction, the all assertion should be an any assertion.

@mroeschke mroeschke added this to the 3.0 milestone Nov 11, 2025
@mroeschke mroeschke added the Testing pandas testing functions or related to the test suite label Nov 11, 2025
exp = Series(period_range("2000-01-01", periods=10, freq="D"))
exp._values.view("i8")[key] = NaT._value
assert exp[key] is NaT or all(x is NaT for x in exp[key])
assert exp[key] is NaT or any(x is NaT for x in exp)
Copy link
Member

Choose a reason for hiding this comment

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

if this was passing before, does it suggest that exp[key] is NaT is always the case?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. Yeah I think then we can remove the or condition here

Copy link
Member Author

Choose a reason for hiding this comment

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

So after removing the condition in ae2376a some jobs are failing to appears we do hit the second condition somtimes

Copy link
Member

Choose a reason for hiding this comment

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

i guess depending on key, exp[key] could be a scalar or non-scalar. In the non-scalar case, i would expect the existing all assertion to hold. why am i wrong?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah OK I see now. I missed that key can be a slice.

@mroeschke mroeschke closed this Dec 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Testing pandas testing functions or related to the test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants