-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
Period.asfreq broken for freq=MS #5332
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
Comments
You've been looking through, right? Do you know where to find the fix? (I'm pretty sure it's because it lower cases the name when trying to get the alias from the period string). Also, this was in v0.12.0 as well, so wasn't introduced by recent refactoring. |
Agreed that it was not recently added. I did actually test that. I can look into where the lower case call is. |
It looks like the |
So this is slightly more complicated:
would it be okay to remove |
Come to think of it, I am not sure that MS make sense as a period frequency. That being said it seems safest to not allow MS to be used to mean milliseconds as that is ambiguous. It's pretty easy to make MS unknown rather than mapped to milliseconds. Thoughts? |
@cancan101 this is very odd - I don't see the error anymore. I ran your code, got the KeyError, but now I'm working on the same master as before and I get this: In [2]: pd.Period("2013").asfreq(freq="MS", how="S")
Out[2]: Period('2013-01-01 00:00:00.000', 'L') and this In [3]: pd.Period("2013").asfreq(freq="MS")
Out[3]: Period('2013-12-31 23:59:59.999', 'L') |
That is very odd. I have a simple fix for this BTW. On Fri, Oct 25, 2013 at 8:54 PM, Jeff Tratner [email protected]:
|
Let me know if that's OK and I will send PR. |
closed via #5340 |
This works:
but this does not:
it seems to be treating the capital
MS
as lower casems
, meaning millisecond. Observer similar results:The text was updated successfully, but these errors were encountered: