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

Adding default coordinates to Datasets with missing image coords #605

Merged
merged 6 commits into from May 21, 2021

Conversation

jlstevens
Copy link
Collaborator

This PR aims to address #603 by generating default image coordinates when they are missing.

Before the PR, using the example given in #603:

With the PR:

So far I've only thought about the Dataset branch, to finish this PR I'll need to handle the DataArray case as well.

@jlstevens
Copy link
Collaborator Author

Some datashader related tests seem to be failing on Python 3. Python 2 is passing because:

test_plot_resolution_0_image (hvplot.tests.testoperations.TestChart2D) ... SKIP: xarray or datashader not available

@jlstevens
Copy link
Collaborator Author

The datashader test failures are not being caused by this PR so that is a separate issue to fix. Restoring the original commit.

@rabernat
Copy link

rabernat commented May 4, 2021

Amazing, thanks for your work on this!

I would make sure to test many possible combinations here (no coordinates, some coordinates, dask, no dask, different ndims, etc.) The example I reported in #603 was just one fail case. Every different scenario seemed to fail with a slightly different error message, and unfortunately I did not take the time to document all of them.

hvplot/converter.py Outdated Show resolved Hide resolved
@@ -1026,6 +1026,12 @@ def __call__(self, kind, x, y):
elif self.datatype == 'xarray':
import xarray as xr
if isinstance(data, xr.Dataset):
if kind == 'image':
Copy link
Member

Choose a reason for hiding this comment

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

How about quadmeshes?

Copy link
Member

Choose a reason for hiding this comment

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

And contours and contourf?

Copy link
Collaborator Author

@jlstevens jlstevens May 4, 2021

Choose a reason for hiding this comment

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

Yup, all need to be handled. PR is still WIP and not ready for review. The first commit was simply tackling the particular example reported in the issue.

Co-authored-by: Philipp Rudiger <prudiger@anaconda.com>
@jlstevens
Copy link
Collaborator Author

I would make sure to test many possible combinations here (no coordinates, some coordinates, dask, no dask, different ndims, etc.)

Absolutely! I'll make sure there are unit tests for these cases.

@jlstevens jlstevens force-pushed the jlstevens/default_image_coords branch from ecbd91c to fc500d3 Compare May 4, 2021 13:58
setup.py Outdated Show resolved Hide resolved
@philippjfr
Copy link
Member

Going to merge this as a first cut, please do follow up on all the other cases but I'm releasing 0.7.2 tonight so wanted to get this in.

@philippjfr philippjfr merged commit aa2e9e2 into master May 21, 2021
@maximlt maximlt deleted the jlstevens/default_image_coords branch November 18, 2021 08:33
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 this pull request may close these issues.

None yet

3 participants