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

xarray support in imshow #2166

Merged
merged 25 commits into from Mar 23, 2020
Merged

xarray support in imshow #2166

merged 25 commits into from Mar 23, 2020

Conversation

emmanuelle
Copy link
Contributor

I'm planning to complete the docstring and add examples in the tutorials, but I would like first to discuss whether this xarray support should rather go here in imshow or/and in a px.matrix_heatmap / px.heatmap function which is yet to be written (but could be adapted from the imshow code). @nicolaskruchten interested in your thoughts here :-).

@emmanuelle
Copy link
Contributor Author

Oh and I will fix the CI too of course (add xarray to CI deps). My hesitation here is in particular for arrays generated by datashader: since their dimensions are sometimes heterogeneous, it does not always make sense to plot them with square pixels.

@emmanuelle emmanuelle changed the title [WIP] xarray support in imshow xarray support in imshow Feb 10, 2020
@nicolaskruchten
Copy link
Member

This looks great, but per semver rules belongs in 4.6 :)

@emmanuelle
Copy link
Contributor Author

no worries :-). Anything to change ?

@emmanuelle
Copy link
Contributor Author

Now that the patch release is out, I think this one is ready for review :-).

if "title" not in layout_patch and args["template"].layout.margin.t is None:
layout_patch["margin"] = {"t": 60}
fig = go.Figure(data=trace, layout=layout)
fig.update_layout(layout_patch)
if img_is_xarray:
Copy link
Member

Choose a reason for hiding this comment

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

I actually think we can do this for np inputs also, no, given the labels dict? We can just default x_label to x etc

@nicolaskruchten
Copy link
Member

@emmanuelle LMK what you think of my changes... missing additional tests, docstring and docs but I will add them if you agree.

Basically what I've done is generalized x and y a little bit so that a user can pass them in even with a normal numpy array for img. In essence, an alternative way to look at this PR with my changes is that it first adds user-controllable x, y and labels to imshow(), and then adds special handling for xarrays to prepopulate those values if unspecified ;)

@emmanuelle
Copy link
Contributor Author

Thank you this looks great ! Looks like you added the tests already, 💃 !

@nicolaskruchten nicolaskruchten merged commit 60e3666 into master Mar 23, 2020
@emmanuelle emmanuelle deleted the imshow-xarray branch March 27, 2020 16:27
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

2 participants