-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
Allow float window when using .rolling #12714
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
Conversation
@@ -1354,7 +1354,9 @@ def _get_center_of_mass(com, span, halflife, alpha): | |||
|
|||
|
|||
def _offset(window, center): | |||
if not com.is_integer(window): | |||
if com.is_float(window): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs to raise if its not an integer for Rolling and sub-classes, or int/array of ints for Window
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jreback So pd.DataFrame(np.arange(10)).rolling(2.).median()
should work, while pd.DataFrame(np.arange(10)).rolling(2., center=True).median()
should fail with argument validation (only difference is center=True
)? Can you confirm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
neither would wifi now should be an int if not it's an error
I just noted in the associated issue that that I think it would be better to raise than coerce integers. It could be OK to accept floats if |
I agree we should just raise on non-int. I think was an oversight in the initial impl. yes its safe if |
competing PR is #12718 |
@gliptak want to update (also see my comments on the competeting PR). |
git diff upstream/master | flake8 --diff
#12669