issue_comments: 449555459

This data as json

html_url issue_url id node_id user created_at updated_at author_association body reactions performed_via_github_app issue
https://github.com/pydata/xarray/pull/2593#issuecomment-449555459 https://api.github.com/repos/pydata/xarray/issues/2593 449555459 MDEyOklzc3VlQ29tbWVudDQ0OTU1NTQ1OQ== 8708062 2018-12-22T08:39:59Z 2018-12-22T08:39:59Z CONTRIBUTOR

Glad to see that I'm not the only one getting different results. And I agree (biased as I am) that the additional bin at the end of resampled time series is superfluous.

If pandas master with the altered resampling logic will be the definitive version going forward, should development of CFTimeIndex resampling be suspended until this version of pandas master is released and xarray uses it as a dependency?

On a somewhat related note, looking over the latest pandas master resample.py made me realize that https://github.com/pydata/xarray/blob/4317c697900c80604dee793ffc1186e5c57a03fd/xarray/core/resample_cftime.py#L114-L118 is now wrong due to changes made 16 days ago (https://github.com/pandas-dev/pandas/issues/24127) . Since non-integer offset frequencies are not supported by BaseCFTimeOffset, (is_day and offset.n == 1 should just be is_day), which can be further simplified to: python if isinstance(offset, CFTIME_TICKS): return _adjust_dates_anchored(first, last, offset, closed=closed, base=base)

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   387924616