Skip to content
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

auto_combine warnings with open_mfdataset #3091

Closed
dcherian opened this issue Jul 10, 2019 · 6 comments · Fixed by #3101
Closed

auto_combine warnings with open_mfdataset #3091

dcherian opened this issue Jul 10, 2019 · 6 comments · Fixed by #3101

Comments

@dcherian
Copy link
Contributor

I'm seeing this warning when using open_mfdataset(list_of_files) on master.

/gpfs/u/home/dcherian/python/xarray/xarray/backends/api.py:783: FutureWarning: In xarray version 0.13 `auto_combine` will be deprecated.
/gpfs/u/home/dcherian/python/xarray/xarray/backends/api.py:783: FutureWarning: The datasets supplied have global dimension coordinates. You may want
to use the new `combine_by_coords` function (or the
`combine='by_coords'` option to `open_mfdataset` to order the datasets
before concatenation. Alternatively, to continue concatenating based
on the order the datasets are supplied in in future, please use the
new `combine_nested` function (or the `combine='nested'` option to
open_mfdataset).

I don't think we should be issuing this warning when using open_mfdataset with default kwargs. Also there's a typo: "in in future"

@TomNicholas
Copy link
Member

TomNicholas commented Jul 11, 2019

I don't think we should be issuing this warning when using open_mfdataset with default kwargs

Why? Do you feel it's not clear enough? The warning is to indicate that the current behaviour of open_mfdataset will change slightly, and to not be caught by surprise then a combine argument should now be supplied.

To understand why a deprecation cycle would be required read from here (but it is long and confusing and the function names change so happy to explain the logic again if you want).

@dcherian
Copy link
Contributor Author

When calling open_mfdataset it's not clear why I'm getting a warning about auto_combine (as a user). The second warning seems OK.

@max-sixty
Copy link
Collaborator

I don't use the function but so as an outsider: all the information is there, but it took me a few reads through to understand it. Depending on how widely used this is, I think something like this would be clearer (h/t to similar pandas' warnings):

The default behavior of open_mfdataset will change in xarray [0.14] for datasets with global dimension coordinates: [datasets will be combined by their coords {maybe better explanation, maybe link}]
To retain the existing behavior, pass combine='nested'. To use future default behavior, pass combine='by_coords'.

@TomNicholas
Copy link
Member

Okay fair points.

From an implementation point of view it makes sense to raise one warning just saying that function will be deprecated/have deprecated behaviour, then immediately raise further warnings with details as to what to do. (Otherwise you'll have to start secretly passing partial warning message strings into auto_combine, which is possible but seems clunky.) If I do that so it specifically refers to open_mfdataset in the first warning, then gives a (better) generic explanation in the second, do you think that would be sufficient?

@max-sixty
Copy link
Collaborator

I think that would be OK.
Another option - more work but better - is to raise a single warning in open_mfdataset and suppress downstream warnings (we have a suppress contextmanager IIRC)

I'd generally weigh towards less tax on people making changes, even if the user experience is a bit worse temporarily. So in this case I would vote to do whatever you're happy to do quickly @TomNicholas

@TomNicholas
Copy link
Member

Okay have a look at PR #3101.

It will now issue a function-specific warning depending on if you enter from open_mfdataset or auto_combine, e.g.

"""In xarray version 0.13 the default behaviour of `open_mfdataset`
will change. To retain the existing behavior, pass
combine='nested'. To use future default behavior, pass
combine='by_coords'. See http://xarray.pydata.org/en/stable/combining.html#combining-multi"""

It links to this page of the docs. This is controlled by a hidden from_openmfds kwarg.

Then it issues the same set of warnings as before, whose exact form depends on the structure of the users input.

This could be improved further, but I think this should already be much clearer than before.

weiji14 added a commit to weiji14/deepbedmap that referenced this issue Jul 19, 2019
Was getting some deprecation warnings with xarray being upgraded from 0.12.1 to 0.12.3. Specifically, the open_mfdatset used in deepbedmap.get_image_with_bounds warns of an upcoming functional change in 0.13. Warning is silenced by setting a `combine="nested"` argument that replicates the old behaviour. See also pydata/xarray#3091.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants