BUG: support fused types in roll_min/max #12373
State: closed

- Closes #12373
- Added test_rolling_min_max_types to test_window.py
- passes `git diff upstream/master | flake8 --diff`

Ran the performance tests with no changes detected:

$ asv continuous -b 'stat_op.*rolling' upstream/master GH12373
· Creating environments
· Discovering benchmarks
·· Uninstalling from py2.7-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
·· Building for py2.7-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt..................................................
·· Installing into py2.7-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt..
· Running 18 total benchmarks (2 commits \* 1 environments \* 9 benchmarks)
[  0.00%] · For pandas commit hash 170fb27a:
[  0.00%] ·· Building for py2.7-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt.......................................................
[  0.00%] ·· Benchmarking py2.7-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
[  5.56%] ··· Running stat_ops.stats_rolling_mean.time_rolling_kurt                                                                                                                                  11.39ms
[ 11.11%] ··· Running stat_ops.stats_rolling_mean.time_rolling_max                                                                                                                                    7.48ms
[ 16.67%] ··· Running stat_ops.stats_rolling_mean.time_rolling_mean                                                                                                                                   7.22ms
[ 22.22%] ··· Running stat_ops.stats_rolling_mean.time_rolling_median                                                                                                                               114.21ms
[ 27.78%] ··· Running stat_ops.stats_rolling_mean.time_rolling_min                                                                                                                                    7.55ms
[ 33.33%] ··· Running stat_ops.stats_rolling_mean.time_rolling_skew                                                                                                                                  11.35ms
[ 38.89%] ··· Running stat_ops.stats_rolling_mean.time_rolling_std                                                                                                                                    9.13ms
[ 44.44%] ··· Running stat_ops.stats_rolling_mean.time_rolling_sum                                                                                                                                    6.93ms
[ 50.00%] ··· Running stat_ops.stats_rolling_mean.time_rolling_var                                                                                                                                    7.98ms
[ 50.00%] · For pandas commit hash 56e285a6:
[ 50.00%] ·· Building for py2.7-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt.......................................................
[ 50.00%] ·· Benchmarking py2.7-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
[ 55.56%] ··· Running stat_ops.stats_rolling_mean.time_rolling_kurt                                                                                                                                  11.66ms
[ 61.11%] ··· Running stat_ops.stats_rolling_mean.time_rolling_max                                                                                                                                    8.68ms
[ 66.67%] ··· Running stat_ops.stats_rolling_mean.time_rolling_mean                                                                                                                                   7.48ms
[ 72.22%] ··· Running stat_ops.stats_rolling_mean.time_rolling_median                                                                                                                               107.71ms
[ 77.78%] ··· Running stat_ops.stats_rolling_mean.time_rolling_min                                                                                                                                    8.74ms
[ 83.33%] ··· Running stat_ops.stats_rolling_mean.time_rolling_skew                                                                                                                                  11.53ms
[ 88.89%] ··· Running stat_ops.stats_rolling_mean.time_rolling_std                                                                                                                                    9.25ms
[ 94.44%] ··· Running stat_ops.stats_rolling_mean.time_rolling_sum                                                                                                                                    6.97ms
[100.00%] ··· Running stat_ops.stats_rolling_mean.time_rolling_var                                                                                                                                    8.24msBENCHMARKS NOT SIGNIFICANTLY CHANGED.


Labels:
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations

Comments:
ideally if you can run the perf tests that cover rolling min/max and see if anything is changing (prob not).

We prob need to expand the dtypes that are tested as well.

will be in a slight conflict with #12376, IOW we don't need to upcast for min/max (but do for other algos). So need a flag for that somewhere. (until everything is converted like this)

I feel a bit embarrassed for not completely reading the issue and noticing that someone already did a PR. I think my fix is a bit more comprehensive. I’ll take a look this week and try to merge.

From: Jeff Reback [mailto:notifications@github.com]
Sent: Saturday, February 27, 2016 10:43 AM
To: pydata/pandas
Cc: Joshua Storck
Subject: Re: [pandas] Bugfix for #12373 (#12481)

will be in a slight conflict with #12376https://github.com/pydata/pandas/pull/12376, IOW we don't need to upcast for min/max (but do for other algos). So need a flag for that somewhere.

—
Reply to this email directly or view it on GitHubhttps://github.com/pydata/pandas/pull/12481#issuecomment-189669738.

@joshuastorck no worries. I think the other fix has some comprehensive tests which is good and will move the needle. however your fix on top is ultimately better as the fused types are more general.

@joshuastorck Your fix makes more sense than just force every roll_<function> to only work on float64. I was thinking about this but I am not very familiar with Cython so I just haven't touch the Cython code.
Glad to see that someone will do this for making pandas better

@joshuastorck can you rebase/update

I'm closing this in favor of #12595 

@joshuastorck ok!