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

XArrayInterface improvements: dimension units and labels #2431

Merged
merged 4 commits into from
Mar 15, 2018

Conversation

drs251
Copy link
Contributor

@drs251 drs251 commented Mar 11, 2018

This addresses the first two points in #2319. As @philippjfr suggested, it's relatively straightforward to read units and dimension labels from xarray and attach them to the appropriate key and value dimensions (as far as I can tell, the attributes units and long_name are a standard for netcdf files). I could not reproduce the the problem with the swapped kdims for xarray.DataArray vs. xarrayDataset, but I added a test to guard against it.

I would like to try to add some stuff to this PR with your input (see #2319):

  • Support for dimensions without coordinates
  • Perhaps a better way to deal with non-coordinate dimensions
  • When everything else is done: some extra documentation.

…nsion labels for all key and value dimensions, added test to make sure xarray.Datasets and xarray.DataArrays are treated equivalently
@philippjfr
Copy link
Member

Looks good so far and the other suggestions sound good too although I suspect it might get tricky.

@drs251
Copy link
Contributor Author

drs251 commented Mar 12, 2018

I'm tempted to agree; the first two points should probably be implemented together, and the second one could be a real head scratcher. I'll try to put some thought into it, but if it's too ambitious, I'll just add the documentation I had in mind and leave the issue open for another time. Anyway, I would like to get this in the next release, if possible. Any ideas when that will be?

@drs251 drs251 changed the title XArrayInterface improvements: will now attempt to read units and dime… XArrayInterface improvements: units and dimensions Mar 12, 2018
@drs251 drs251 changed the title XArrayInterface improvements: units and dimensions XArrayInterface improvements: dimension units and labels Mar 12, 2018
@philippjfr
Copy link
Member

philippjfr commented Mar 12, 2018

We were hoping to have a feature freeze at the end of this week, with a release following a few days later.

@drs251
Copy link
Contributor Author

drs251 commented Mar 12, 2018

OK, then I'll probably concentrate on the docs for now.

@drs251
Copy link
Contributor Author

drs251 commented Mar 15, 2018

@philippjfr OK, I added a simple example of how visualize an xarray Dataset. I also found that something similar already exists for GeoViews, so I added some links. I don't think I'll have time right now to work on the other ideas I have for XArrayInterface, so I would consider this PR to be finished, but I'll leave issue #2319 open and hopefully I'll manage to come back to it sometime.

@philippjfr
Copy link
Member

At a quick glance this looks great! Would you mind clearing the notebook and committing that. We generally don't store notebooks with output and it will make it easier to review.

"cell_type": "markdown",
"metadata": {},
"source": [
"Note, that this approach immediately converts all available data to images, which will take up a lot of RAM for large datasets. For these situations, consider using a [DynamicMap](./06-Live_Data.ipynb) in conjunction with [xarray's dask support](http://xarray.pydata.org/en/stable/dask.html) instead.\n",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe mention the dynamic argument to .to here, it will generate a DynamicMap for you.

@philippjfr
Copy link
Member

One minor comment, otherwise looks good to merge.

@drs251
Copy link
Contributor Author

drs251 commented Mar 15, 2018

BTW: Does anyone know how to change the vdim label in this case? I've tried redim(), but it refuses to change existing labels. Directly accessing the dimension also didn't work.

@philippjfr
Copy link
Member

You mean changing a label after it has already been set? It was a deliberate decision not to allow that although @jlstevens was more insistent on that than I was. You can override the existing label like this though:

xr_ds = xr.tutorial.load_dataset("air_temperature")
hv.Dataset(xr_ds, vdims=('air', 'air_temp')).redim(air=('air', 'air_temperature')).vdims[0].label

@drs251
Copy link
Contributor Author

drs251 commented Mar 15, 2018

Sorry, but I still don't get it: I'm absolutely unable to replace the rather long-winded "4xDaily Air temperature at sigma level 995" axis label in the plot within Holoviews. On the other hand, it's not essential for this PR though.

@philippjfr
Copy link
Member

Right, that's a separate issue in any case. I'll play around once merged and if I can reproduce I'll raise an issue.

@philippjfr
Copy link
Member

Thanks for the contribution!

@philippjfr philippjfr merged commit 41e02f1 into holoviz:master Mar 15, 2018
@drs251 drs251 deleted the xarray_units branch March 15, 2018 16:08
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants