Skip to content

Conversation

@gliptak
Copy link
Contributor

@gliptak gliptak commented Mar 25, 2016

#12669

Copy link
Contributor

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

Copy link
Contributor Author

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?

Copy link
Contributor

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

@jreback jreback added Reshaping Concat, Merge/Join, Stack/Unstack, Explode Error Reporting Incorrect or improved errors from pandas labels Mar 25, 2016
@shoyer
Copy link
Member

shoyer commented Mar 25, 2016

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 int(window) == window, but window=2.5 should definitely raise an error.

@jreback
Copy link
Contributor

jreback commented Mar 25, 2016

I agree we should just raise on non-int. I think was an oversight in the initial impl. yes its safe if int(x) == x, but more correct to raise TypeError as @shoyer suggests.

@jreback
Copy link
Contributor

jreback commented Apr 1, 2016

competing PR is #12718

@jreback
Copy link
Contributor

jreback commented Apr 9, 2016

@gliptak want to update (also see my comments on the competeting PR).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Error Reporting Incorrect or improved errors from pandas Reshaping Concat, Merge/Join, Stack/Unstack, Explode

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ERR: better error message on invalid window when using .rolling

3 participants