-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
BUG date_range for Annual interval gives end of year instead of start #9312
Comments
I don't think this is a bug, but a choice at the time the aliases were made. This is clearly laid out in the date offset documentation ("Offset Aliases" section). A year end frequency |
Agreed with @rockg. I'm guessing the year-end by default is common for many fields, so we can't really say that year-start frequency is more intuitive. Plus |
Well it is also inconsistent with As for |
Is left closed more common though (I agree that is is for other python ranges)? But for date stuff, I could see this being the natural behavior (I'm thinking of finance). This had to have been a deliberate choice, and not an implementation detail I think. Anyway, this would be a big break in backwards compatibility, and I don't think that the benefit justifies the break. Do you? If we did do anything, I'd break Usually in these situations I'd say we need better documentation. The offsets docs are clear I think, maybe we should note that in the docstring for date_range / DatetimeIndex? |
I personally agree that start would be a more natural default to have it at the start of the year (I'm in finance and yearly aggregations are most of the time easier to think of as year beginning), but it is too entrenched to change now. The same case could be made for |
Interesting. I wonder why this was the choice originally then. This would be a really big break in comparability though :(. |
I'm sure left-closed is common usage in Python precisely because it matches the behaviour of I'm not sure of the origin of this behaviour, it's odd from a physical sciences perspective, especially considering that "W - Weekly" gives you the start of the week - why start of the week but end of the year?? It at least needs to be better explained in the To avoid a backwards incompatible break I would just add new aliases (in any case "A" for "Annual" is odd, when even the docs say "A for Year end frequency", see also #9313). I would create aliases with Y for Year: I would also add for completeness. This would break nothing and give reasonably behaviour for someone new. The A aliases could be kept or depreciated over time (I have no strong opinion on this). As for Month, it should probably also get a ME alias (so at least you can be explicit) but there's otherwise not a good transition story... Maybe just doing a clean break would actually make more sense... |
I like folding in the change here w/ the one from #9313. The month story is tougher to fix. Let's see what others have to say. Sorry about closing the issue early. |
I see no reason why we would change 'A' or 'M' behavior...there are offsets that already do start of the respective frequency. We may not think they are named the best, but there must have been a reason a the time and in the grand scheme it's not a big deal--one time of getting it wrong and then one extra letter. I agree that making the documentation clearer and adding more explicit end frequencies would be a good resolution. |
@rockg certainly no need to change A - but adding Y would still make sense, I think since pandas is still under heavy development it's a good time to adjust this kind of detail. If someone can weight in explaining why that behaviour was chosen it would help the discussion. |
Yes, I agree adding Y makes sense. |
mo believe a lot of this came from here: http://pytseries.sourceforge.net/core.constants.html |
there has been a
|
I think closed only affects the values after they've been aligned at start or end, e.g. In [5]: pd.date_range(datetime.datetime(2006, 1, 1, 0,0,0), periods=2, freq='A', closed='left')
Out[5]:
<class 'pandas.tseries.index.DatetimeIndex'>
[2006-12-31]
Length: 1, Freq: A-DEC, Timezone: None so it makes the The issue here is that the default should be the initial two elements should be year start, |
When Creating an Annual date range from the start of a year, the actual dates generated are for the END of that year, and do not include the datetime provided. You would expect to get a range starting from
start_date
and a series of subsequent dates one year apartFor reference, using
dateutils.rrule
gives intuitive behaviorThe text was updated successfully, but these errors were encountered: