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.

Sign up for GitHub

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

Jump to bottom

Support plots that use by with rasterize with hv.ImageStack #1132

Merged
merged 6 commits into from
Sep 19, 2023
Merged

Support plots that use by with rasterize with hv.ImageStack #1132

merged 6 commits into from
Sep 19, 2023

Conversation

ahuang11
Copy link
Collaborator

@ahuang11 ahuang11 commented Sep 9, 2023 •
edited
Loading

Alongside holoviz/holoviews#5751

import pandas as pd
import hvplot.pandas
import holoviews as hv
hv.extension('bokeh')

df = pd.DataFrame(
    {
        "time": [1, 2, 3, 4, 5, 6, 7, 8, 9, 10],
        "value": [1, 2, 3, 4, 5, 6, 7, 8, 9, 10],
        "by": ["d", "d", "d", "a", "a", "b", "b", "b", "b", "b"],
    }
)

df.hvplot(
    x="time",
    y="value",
    by="by",
    rasterize=True,
    color_key="RdBu_r"
)
image

colorbar is not yet supported in HoloViews.

@ahuang11 ahuang11 changed the title WIP overlaying rasterized WIP overlaying rasterized plots incorrect colormap and wrong colorbar Sep 9, 2023
@ahuang11 ahuang11 changed the title WIP overlaying rasterized plots incorrect colormap and wrong colorbar Overlaying rasterized plots Sep 9, 2023
@ahuang11 ahuang11 changed the title Overlaying rasterized plots Support by with rasterize plots for ImageStack Sep 9, 2023
@ahuang11 ahuang11 changed the title Support by with rasterize plots for ImageStack Support plots that use by with rasterize with hv.ImageStack Sep 9, 2023
hoxbro
hoxbro reviewed Sep 9, 2023
View reviewed changes
Copy link
Member

@hoxbro hoxbro left a comment

Choose a reason for hiding this comment

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

When running the code (with holoviz/holoviews#5751), I get the following plot. I suspect some custom handling of the hover tool is needed.

image

hvplot/converter.py Outdated Show resolved Hide resolved
@maximlt maximlt added this to the 0.9.0 milestone Sep 11, 2023
@maximlt
Copy link
Member

maximlt commented Sep 11, 2023

Just noting, we need to open an issue to record that this needs to be documented when HoloViews is released with the new feature this PR depends on.

@ahuang11
Copy link
Collaborator Author

I am confused, what needs to be documented?

@maximlt
Copy link
Member

maximlt commented Sep 11, 2023

Sorry for the lack of context. rasterize together with by should certainly be documented, Simon said he already has a notebook in preparation. I'll open an issue.

@maximlt maximlt mentioned this pull request Sep 11, 2023
Open
@ahuang11
Copy link
Collaborator Author

When running the code (with holoviz/holoviews#5751), I get the following plot. I suspect some custom handling of the hover tool is needed.

image

Seems like this needs to be fixed in holoviews
image

@hoxbro
Copy link
Member

hoxbro commented Sep 12, 2023

I have left a comment in the holoviews issue about the hover tool. I'm also a bit surprised that the colorbar stops at 0.8.

hoxbro
hoxbro reviewed Sep 19, 2023
View reviewed changes
hvplot/converter.py Outdated Show resolved Hide resolved
@ahuang11 @hoxbro
Update hvplot/converter.py
c892111 
Co-authored-by: Simon Høxbro Hansen <simon.hansen@me.com>
hoxbro
hoxbro approved these changes Sep 19, 2023
View reviewed changes
Copy link
Member

@hoxbro hoxbro left a comment

Choose a reason for hiding this comment

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

If this is ready, you can merge it.

hvplot/tests/testoperations.py
@@ -194,6 +195,11 @@ def test_datashade_rescale_discrete_levels_default_True(self):
actual = plot.callback.inputs[0].callback.operation.p['rescale_discrete_levels']
assert actual is expected

def test_rasterize_by(self):
Copy link
Member

Choose a reason for hiding this comment

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

This should also have a version check, but let us ignore it if all tests pass.

Copy link
Member

Choose a reason for hiding this comment

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

Yep for us it works well but sometimes distros that re-package our tools complain about this kind of stuff (and probably just patch things).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok I'll add a check in another PR

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

#1144

@ahuang11 ahuang11 merged commit 3700ae2 into main Sep 19, 2023
7 checks passed
@ahuang11 ahuang11 deleted the overlay_imagestack branch September 19, 2023 15:31
ahuang11 added a commit that referenced this pull request Sep 27, 2023
@ahuang11 @hoxbro
Support plots that use by with rasterize with hv.ImageStack (#1132)
2cd9a10 
* WIP overlaying rasterized

* Clean up

* Add rasterize to check for categorical

* Add test

* Add version check

* Update hvplot/converter.py

Co-authored-by: Simon Høxbro Hansen <simon.hansen@me.com>

---------

Co-authored-by: Simon Høxbro Hansen <simon.hansen@me.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maximlt maximlt maximlt left review comments

@hoxbro hoxbro hoxbro approved these changes

@philippjfr philippjfr Awaiting requested review from philippjfr

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.9.0
Development

Successfully merging this pull request may close these issues.
3 participants
@ahuang11 @maximlt @hoxbro