issue_comments

Thanks for the smooth review process @spencerkclark @keewis

LGTM too. Thanks @aaronspring

Hello @aaronspring! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2020-11-23 16:02:32 UTC

Ah I need to account for the docstring examples with only one or two items, maybe dropping freq then

Thanks @keewis I didn't realize this was also happening in CI. Yeah, the version is definitely the problem: cftime 1.0.4.2 py36hc1659b7_0 conda-forge My vote for now would be to skip these tests on versions less than 1.1.0. Perhaps we could also think about adding a nicer error message if the version is not up-to-date enough.

~~I guess our test coverage of infer_freq wasn't good enough to trip this up already.~~ Oops we already have a precedent of skipping the infer_freq tests that require this, e.g.: https://github.com/pydata/xarray/blob/a2192158e3fbb94b2d972ff3e1693fffa65e50be/xarray/tests/test_cftimeindex.py#L1180-L1192

@spencerclark, see the build logs of the py36-min-all-deps CI, we can't use version 1.1.0 unless we bump the version of cftime

freq would be consistent with pandas: ``` In [1]: import pandas as pd

In [2]: pd.date_range("2000", periods=3) Out[2]: DatetimeIndex(['2000-01-01', '2000-01-02', '2000-01-03'], dtype='datetime64[ns]', freq='D') ```

@aaronspring regarding the local test failure -- which version of cftime are you using in your environment? The daysinmonth attribute did not always exist; it was added in version 1.1.0.

the parameter is named freq so I think it should be fine to use that, but I don't have a strong opinion on this

also should it be frequency or freq?

somehow in some envs xarray (but not in others)

Actually, there's no inconsistency here: py38-flaky fails, but it is allowed to fail so we still get a green check mark (#4584 slightly changes that). py36-min-nep18 does not install cftime so it doesn't fail, either. Doctests fails because it creates and prints CFTimeIndex objects with 1 or 2 values (and I think there's also a test that does something similar).

To fix the CI, you might need to fix both the daysinmonth and allow CFTimeIndex objects with less than 3 values.

I also get this error locally. somehow in some envs xarray (but not in others) tries to use daysinmonth. I also know about days_in_month, but just changing that in frequencies.py https://github.com/pydata/xarray/blob/d9ebcafaddf0bb80052d30f43ca5ea18a5c7229f/xarray/coding/frequencies.py#L261 didnt help it.

``` ____________ test_cftimeindex_freq_in_repr[MS-noleap] _______________

freq = 'MS', calendar = 'noleap'

@requires_cftime
@pytest.mark.parametrize("calendar", ["noleap", "360_day"])
@pytest.mark.parametrize("freq", ["1D", "MS"])
def test_cftimeindex_freq_in_repr(freq,calendar):
    index = xr.cftime_range(start="2000", periods=3, freq=freq, calendar=calendar)

  assert f', freq={freq}' in index.__repr__()

/Users/aaron.spring/Coding/xarray/xarray/tests/test_cftimeindex.py:952:

/Users/aaron.spring/Coding/xarray/xarray/coding/cftimeindex.py:342: in repr attrs_str = format_attrs(self) /Users/aaron.spring/Coding/xarray/xarray/coding/cftimeindex.py:263: in format_attrs "freq": f"'{index.freq}'" /Users/aaron.spring/Coding/xarray/xarray/coding/cftimeindex.py:691: in freq return infer_freq(self) /Users/aaron.spring/Coding/xarray/xarray/coding/frequencies.py:97: in infer_freq return inferer.get_freq() /Users/aaron.spring/Coding/xarray/xarray/coding/frequencies.py:132: in get_freq return self._infer_daily_rule() /Users/aaron.spring/Coding/xarray/xarray/coding/frequencies.py:165: in _infer_daily_rule monthly_rule = self._get_monthly_rule() /Users/aaron.spring/Coding/xarray/xarray/coding/frequencies.py:199: in _get_monthly_rule return {"cs": "MS", "ce": "M"}.get(month_anchor_check(self.index))

dates = <[AttributeError("'cftime._cftime.DatetimeNoLeap' object has no attribute 'daysinmonth'",) raised in repr()] CFTimeIndex object at 0x7fca9312b160>

def month_anchor_check(dates):
    """Return the monthly offset string.

    Return "cs" if all dates are the first days of the month,
    "ce" if all dates are the last day of the month,
    None otherwise.

    Replicated pandas._libs.tslibs.resolution.month_position_check
    but without business offset handling.
    """
    calendar_end = True
    calendar_start = True

    for date in dates:
        if calendar_start:
            calendar_start &= date.day == 1

        if calendar_end:

          cal = date.day == date.daysinmonth

E AttributeError: 'cftime._cftime.DatetimeNoLeap' object has no attribute 'daysinmonth'

/Users/aaron.spring/Coding/xarray/xarray/coding/frequencies.py:261: AttributeError ____________ test_cftimeindex_freq_in_repr[MS-360_day] ______________

freq = 'MS', calendar = '360_day'

@requires_cftime
@pytest.mark.parametrize("calendar", ["noleap", "360_day"])
@pytest.mark.parametrize("freq", ["1D", "MS"])
def test_cftimeindex_freq_in_repr(freq,calendar):
    index = xr.cftime_range(start="2000", periods=3, freq=freq, calendar=calendar)

  assert f', freq={freq}' in index.__repr__()

/Users/aaron.spring/Coding/xarray/xarray/tests/test_cftimeindex.py:952:

/Users/aaron.spring/Coding/xarray/xarray/coding/cftimeindex.py:342: in repr attrs_str = format_attrs(self) /Users/aaron.spring/Coding/xarray/xarray/coding/cftimeindex.py:263: in format_attrs "freq": f"'{index.freq}'" /Users/aaron.spring/Coding/xarray/xarray/coding/cftimeindex.py:691: in freq return infer_freq(self) /Users/aaron.spring/Coding/xarray/xarray/coding/frequencies.py:97: in infer_freq return inferer.get_freq() /Users/aaron.spring/Coding/xarray/xarray/coding/frequencies.py:132: in get_freq return self._infer_daily_rule() /Users/aaron.spring/Coding/xarray/xarray/coding/frequencies.py:165: in _infer_daily_rule monthly_rule = self._get_monthly_rule() /Users/aaron.spring/Coding/xarray/xarray/coding/frequencies.py:199: in _get_monthly_rule return {"cs": "MS", "ce": "M"}.get(month_anchor_check(self.index))

dates = <[AttributeError("'cftime._cftime.Datetime360Day' object has no attribute 'daysinmonth'",) raised in repr()] CFTimeIndex object at 0x7fca930cfe10>

def month_anchor_check(dates):
    """Return the monthly offset string.

    Return "cs" if all dates are the first days of the month,
    "ce" if all dates are the last day of the month,
    None otherwise.

    Replicated pandas._libs.tslibs.resolution.month_position_check
    but without business offset handling.
    """
    calendar_end = True
    calendar_start = True

    for date in dates:
        if calendar_start:
            calendar_start &= date.day == 1

        if calendar_end:

          cal = date.day == date.daysinmonth

E AttributeError: 'cftime._cftime.Datetime360Day' object has no attribute 'daysinmonth'

/Users/aaron.spring/Coding/xarray/xarray/coding/frequencies.py:261: AttributeError ===================================================================================== short test summary info ===================================================================================== FAILED xarray/tests/test_cftimeindex.py::test_cftimeindex_freq_in_repr[1D-noleap] - AttributeError: 'cftime._cftime.DatetimeNoLeap' object has no attribute 'daysinmonth' FAILED xarray/tests/test_cftimeindex.py::test_cftimeindex_freq_in_repr[1D-360_day] - AttributeError: 'cftime._cftime.Datetime360Day' object has no attribute 'daysinmonth' FAILED xarray/tests/test_cftimeindex.py::test_cftimeindex_freq_in_repr[MS-noleap] - AttributeError: 'cftime._cftime.DatetimeNoLeap' object has no attribute 'daysinmonth' FAILED xarray/tests/test_cftimeindex.py::test_cftimeindex_freq_in_repr[MS-360_day] - AttributeError: 'cftime._cftime.Datetime360Day' object has no attribute 'daysinmonth' ======================================================================================== 4 failed in 2.58s =========================================================================== ```