issue_comments

In 6d08d3b I made changes to set_options to only fill the self.old dictionary with the old values for the keyword arguments specified in the constructor, and then had the exit method only reset the values that were temporarily set before. The win-32 build now passes, and we no longer have the undesirable warning behavior shown above. Do you think that was a reasonable fix?

This looks like a reasonable fix to me

Thanks @shoyer -- that was a bit puzzling but I think I sorted it out. I haven't done a careful diagnosis, but my hunch is that the previous build was running into this issue: https://github.com/pytest-dev/pytest/issues/2917. A fix was merged a few weeks ago, but the Python 2.7 32-bit build is/was using a relatively old version of pytest (3.5 versus the most recent 3.9.3).

In the previous build, the FutureWarning for setting the enable_cftimeindex option was being emitted earlier in test_backends.py in the test_open_mfdataset_manyfiles test (before we were watching for it in the options tests). It was being raised there, because upon calling set_options.__exit__, every option (not just the options that were set in a with statement) would be reset to its global value. For example doing something like this would emit the warning (which even outside of this minor testing issue, I think is undesirable): ``` In [1]: import xarray

In [2]: with xarray.set_options(display_width=40): ...: pass ...: /Users/spencerclark/xarray-dev/xarray/xarray/core/options.py:49: FutureWarning: The enable_cftimeindex option is now a no-op and will be removed in a future version of xarray. FutureWarning) `` In 6d08d3b I made changes toset_optionsto only fill theself.olddictionary with the old values for the keyword arguments specified in the constructor, and then had theexit` method only reset the values that were temporarily set before. The win-32 build now passes, and we no longer have the undesirable warning behavior shown above. Do you think that was a reasonable fix?

I don't know why this failed in one of the Appveyor builds: ``` ================================== FAILURES =================================== _____ test_enable_cftimeindex _______ def test_enable_cftimeindex(): with pytest.raises(ValueError): xarray.set_options(enable_cftimeindex=None) with pytest.warns(FutureWarning, match='no-op'): with xarray.set_options(enable_cftimeindex=True):

          assert OPTIONS['enable_cftimeindex']

E Failed: DID NOT WARN. No warnings of type (<type 'exceptions.FutureWarning'>,) was emitted. The list of emitted warnings is: []. xarray\tests\test_options.py:38: Failed ```

Should we add nice error messages to resample and plot telling the user what to do if they try those two with cftimeindex?