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

DEPR: enforce not-automatically aligning in DataFrame comparisons #49946

Merged
merged 6 commits into from
Dec 1, 2022

Conversation

jbrockmendel
Copy link
Member

  • closes #xxxx (Replace xxxx with the GitHub issue number)
  • Tests added and passed if fixing a bug or adding a new feature
  • All code checks passed.
  • Added type annotations to new arguments/methods/functions.
  • Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

Changes look good to me - just got a comment, and wanted to double-check about the issue number in the whatsnew

Comment on lines +310 to +313
box = box_with_array

ser = Series([Timestamp("2000-01-29 01:59:00"), Timestamp("2000-01-30"), NaT])
ser = tm.box_expected(ser, box_with_array)
ser = tm.box_expected(ser, box)
Copy link
Member

Choose a reason for hiding this comment

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

just for my understanding, is this purely cosmetic? (no objections anyway)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it’s a pattern we use elsewhere

doc/source/whatsnew/v2.0.0.rst Outdated Show resolved Hide resolved
@mroeschke mroeschke added the Deprecate Functionality to remove in pandas label Nov 29, 2022
MarcoGorelli
MarcoGorelli approved these changes Nov 29, 2022
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

Ah wait, the related ASV checks are failing

2022-11-29T00:54:09.9434614Z [  1.81%] ··· ...thSeriesAxis.time_frame_op_with_series_axis1        6/12 failed
2022-11-29T00:54:09.9435374Z [  1.81%] ··· ========== ==========
2022-11-29T00:54:09.9435738Z                 opname             
2022-11-29T00:54:09.9436168Z               ---------- ----------
2022-11-29T00:54:09.9436526Z                   eq       failed  
2022-11-29T00:54:09.9437238Z                   ne       failed  
2022-11-29T00:54:09.9437518Z                   lt       failed  
2022-11-29T00:54:09.9437736Z                   le       failed  
2022-11-29T00:54:09.9437960Z                   ge       failed  
2022-11-29T00:54:09.9438183Z                   gt       failed  
2022-11-29T00:54:09.9438611Z                  add      12.0±0ms 
2022-11-29T00:54:09.9438899Z                  sub      11.8±0ms 
2022-11-29T00:54:09.9439204Z                truediv    11.8±0ms 
2022-11-29T00:54:09.9439479Z                floordiv   42.0±0ms 
2022-11-29T00:54:09.9439765Z                  mul      11.6±0ms 
2022-11-29T00:54:09.9440049Z                  pow      31.0±0ms 
2022-11-29T00:54:09.9440264Z               ========== ==========

Good to have fixed the flakiness, I'd got so used to seeing that check fail I was going to just restart it, but this time it's not a false positive

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@MarcoGorelli MarcoGorelli added this to the 2.0 milestone Nov 30, 2022
@mroeschke mroeschke merged commit fc91090 into pandas-dev:main Dec 1, 2022
@mroeschke
Copy link
Member

Thanks @jbrockmendel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecate Functionality to remove in pandas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants