Skip to content

Revert "REF: remove special casing from Index.equals (always dispatchto subclass) (#35330)" #38560

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 18, 2020
Merged

Revert "REF: remove special casing from Index.equals (always dispatchto subclass) (#35330)" #38560

merged 1 commit into from
Dec 18, 2020

Conversation

simonjayhawkins
Copy link
Member

This reverts commit 0b90685.

       before           after         ratio
     [54682234]       [05c97adb]
     <master>         <revert-35330>
-         293±5ms       3.75±0.2μs     0.00  index_object.IndexEquals.time_non_object_equals_multiindex

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE INCREASED.

       before           after         ratio
     [54682234]       [05c97adb]
     <master>         <revert-35330>
-      1.44±0.1ms         800±20μs     0.56  reindex.LevelAlign.time_reindex_level

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE INCREASED.

@simonjayhawkins simonjayhawkins added Performance Memory or execution speed performance Regression Functionality that used to work in a prior pandas version labels Dec 18, 2020
@simonjayhawkins simonjayhawkins added this to the 1.2 milestone Dec 18, 2020
@simonjayhawkins
Copy link
Member Author

more benchmark results

       before           after         ratio
     [54682234]       [05c97adb]
     <master>         <revert-35330>
-     2.97±0.09μs      2.67±0.02μs     0.90  index_object.Indexing.time_slice_step('Float')
-        554±30ns         497±10ns     0.90  index_object.Range.time_get_loc_inc
-      8.56±0.1μs       7.65±0.2μs     0.89  index_object.Indexing.time_get_loc_sorted('Float')
-      2.94±0.2μs      2.63±0.02μs     0.89  index_object.Indexing.time_slice('Float')
-        330±10μs          294±3μs     0.89  index_object.IntervalIndexMethod.time_intersection(1000)
-     5.40±0.09μs      4.79±0.07μs     0.89  index_object.Indexing.time_get_loc_sorted('Int')
-     1.02±0.03ms          898±3μs     0.88  index_object.IntervalIndexMethod.time_intersection_both_duplicate(1000)
-      5.42±0.3ms      4.75±0.01ms     0.87  index_object.SetOperations.time_operation('int', 'symmetric_difference')
-      14.4±0.5ms       12.4±0.1ms     0.86  index_object.Range.time_iter_inc
-      10.1±0.8ms      8.71±0.06ms     0.86  index_object.IntervalIndexMethod.time_intersection(100000)
-      10.3±0.9μs       8.83±0.2μs     0.86  index_object.Indexing.time_get_loc_non_unique_sorted('Int')
-      3.24±0.2μs      2.77±0.02μs     0.85  index_object.Indexing.time_slice('Int')
-         108±6ms       90.6±0.5ms     0.84  index_object.IntervalIndexMethod.time_intersection_both_duplicate(100000)
-        329±10μs          274±3μs     0.83  index_object.IntervalIndexMethod.time_intersection_one_duplicate(1000)
-        25.9±2μs       21.2±0.2μs     0.82  index_object.Indexing.time_get_loc_non_unique_sorted('Float')
-      7.42±0.5ms      5.69±0.07ms     0.77  index_object.SetDisjoint.time_datetime_difference_disjoint
-        16.3±1ms      12.5±0.05ms     0.77  index_object.SetOperations.time_operation('date_string', 'union')
-        4.98±1ms       3.65±0.2ms     0.73  index_object.Indexing.time_get_loc_non_unique('Float')
-        303±20ms       3.86±0.9μs     0.00  index_object.IndexEquals.time_non_object_equals_multiindex

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE INCREASED.

jreback
jreback approved these changes Dec 18, 2020
View reviewed changes
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

lgtm @jbrockmendel if any comments

pandas/core/indexes/base.py
and type(other) is not type(self)
and other.equals is not self.equals
):
if is_object_dtype(self.dtype) and not is_object_dtype(other.dtype):
Copy link
Contributor

Choose a reason for hiding this comment

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

@jbrockmendel

let's make this an elif & put a big note that these are perf sensitive checks (followon)

pandas/core/indexes/base.py
and type(other) is not type(self)
and other.equals is not self.equals
):
if is_object_dtype(self.dtype) and not is_object_dtype(other.dtype):
Copy link
Contributor

Choose a reason for hiding this comment

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

note that this also hits the MultiIndex path

@jbrockmendel
Copy link
Member

LGTM

@simonjayhawkins
Copy link
Member Author

@jreback OK to merge this as a straight revert for backport and then follow-up for master

@jreback jreback merged commit 21b57fa into pandas-dev:master Dec 18, 2020
@simonjayhawkins simonjayhawkins deleted the revert-35330 branch December 18, 2020 19:05
@simonjayhawkins
Copy link
Member Author

@meeseeksdev backport 1.2.x

meeseeksmachine pushed a commit to meeseeksmachine/pandas that referenced this pull request Dec 18, 2020
@simonjayhawkins @meeseeksmachine
Backport PR pandas-dev#38560: Revert "REF: remove special casing from…
3054c45 
… Index.equals (always dispatchto subclass) (pandas-dev#35330)"
@meeseeksmachine meeseeksmachine mentioned this pull request Dec 18, 2020
Merged
simonjayhawkins added a commit that referenced this pull request Dec 18, 2020
@meeseeksmachine @simonjayhawkins
Backport PR #38560: Revert "REF: remove special casing from Index.equ…
2ee1751 
…als (always dispatchto subclass) (#35330)" (#38565)

Co-authored-by: Simon Hawkins <[email protected]>
@jorisvandenbossche jorisvandenbossche mentioned this pull request Dec 20, 2020
Closed
luckyvs1 pushed a commit to luckyvs1/pandas that referenced this pull request Jan 20, 2021
@simonjayhawkins @luckyvs1
Revert "REF: remove special casing from Index.equals (always dispatch…
9901c8c 
… to subclass) (pandas-dev#35330)" (pandas-dev#38560)

This reverts commit 0b90685.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback approved these changes

Assignees
No one assigned
Labels
Performance Memory or execution speed performance Regression Functionality that used to work in a prior pandas version
Projects
None yet
Milestone
1.2
3 participants
@simonjayhawkins @jbrockmendel @jreback