Skip to content

Conversation

@mroeschke
Copy link
Member

@mroeschke mroeschke commented Feb 4, 2021

  • tests added / passed
  • Ensure all linting tests pass, see here for how to run them
  • whatsnew entry

Also I think I stumbled on some bugs along the way; added whatsnew and modified the existing tests where necessary.

$ asv_bench % asv continuous -f 1.1 upstream/master HEAD -b rolling.Pairwise.time_groupby

       before           after         ratio
     [f79468b8]       [6a1e8592]
     <master>         <ref/groupby_rolling_pairwise>
-        299±50ms         178±20ms     0.59  rolling.Pairwise.time_groupby(1000, 'corr', True)
-         272±7ms         157±20ms     0.58  rolling.Pairwise.time_groupby(1000, 'cov', True)
-         262±1ms          146±4ms     0.56  rolling.Pairwise.time_groupby(None, 'cov', True)
-        303±10ms          158±6ms     0.52  rolling.Pairwise.time_groupby(10, 'cov', True)
-        279±20ms        139±0.7ms     0.50  rolling.Pairwise.time_groupby(None, 'corr', True)
-        315±10ms         149±10ms     0.47  rolling.Pairwise.time_groupby(10, 'corr', True)
-         137±2ms       34.0±0.8ms     0.25  rolling.Pairwise.time_groupby(10, 'cov', False)
-         127±5ms         28.2±1ms     0.22  rolling.Pairwise.time_groupby(None, 'cov', False)
-         134±5ms       28.1±0.4ms     0.21  rolling.Pairwise.time_groupby(1000, 'cov', False)
-         135±2ms         27.7±1ms     0.21  rolling.Pairwise.time_groupby(None, 'corr', False)
-        178±10ms         33.7±2ms     0.19  rolling.Pairwise.time_groupby(10, 'corr', False)
-        159±20ms         29.9±2ms     0.19  rolling.Pairwise.time_groupby(1000, 'corr', False)

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE INCREASED.

@mroeschke mroeschke added this to the 1.3 milestone Feb 4, 2021
@mroeschke mroeschke added Performance Memory or execution speed performance Window rolling, ewma, expanding labels Feb 4, 2021
result.index = result_index
return result

def _apply_pairwise(self, target, other, pairwise, func):
Copy link
Contributor

Choose a reason for hiding this comment

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

wow. can you type the input / output args and add a doc-string.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done (a testament to how complex groupby apply is)

@jreback jreback merged commit b2671cc into pandas-dev:master Feb 5, 2021
@jreback
Copy link
Contributor

jreback commented Feb 5, 2021

thanks @mroeschke

@mroeschke mroeschke deleted the ref/groupby_rolling_pairwise branch February 5, 2021 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Performance Memory or execution speed performance Window rolling, ewma, expanding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants