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

Add scalebar support #6002

Merged
merged 25 commits into from
Jun 6, 2024
Merged

Add scalebar support #6002

merged 25 commits into from
Jun 6, 2024

Conversation

hoxbro
Copy link
Member

@hoxbro hoxbro commented Nov 30, 2023

Fixes #5948

I'm not entirely sure what plot classes need to have a scalebar option: all of them or a selective few. For now, I have added it to RGB to start with.

Also, I am not sure how many of the Scalebar options we should have available.

import holoviews as hv
hv.extension("bokeh")

hv.RGB.load_image("5948_pollen.png").opts(scalebar=True, scale_opts={"background_fill_alpha": 1})

image

image_download

@codecov-commenter
Copy link

codecov-commenter commented Nov 30, 2023

Codecov Report

Attention: Patch coverage is 97.84946% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 88.49%. Comparing base (f39e0a0) to head (d6bb610).
Report is 1 commits behind head on main.

Files Patch % Lines
holoviews/plotting/bokeh/element.py 94.11% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6002      +/-   ##
==========================================
+ Coverage   88.48%   88.49%   +0.01%     
==========================================
  Files         323      323              
  Lines       67640    67728      +88     
==========================================
+ Hits        59848    59933      +85     
- Misses       7792     7795       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hoxbro hoxbro added the czi label Nov 30, 2023

from bokeh.models import ScaleBar

kdims = self.hmap.last.kdims
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe not the best way, but I couldn't figure out other ways to get the kdims.

@hoxbro hoxbro marked this pull request as draft November 30, 2023 16:19
@hoxbro
Copy link
Member Author

hoxbro commented Dec 22, 2023

I added a scalebar to the base ElementPlot class, which means that most elements should support having a scalebar.

Example of scalebar with subcordinates_y:

import holoviews as hv
import numpy as np
hv.extension("bokeh")

c1 = hv.Curve(np.random.rand(1000), label='c1').opts(subcoordinate_y=True, scalebar=True, scalebar_location="bottom_left", scalebar_unit="cm")
c2 = hv.Curve(np.random.rand(1000), label='c2').opts(subcoordinate_y=True, scalebar=True, scalebar_location="top_left")

curves = hv.Overlay(c1 * c2)
curves.opts(width=800, height=800)
simplescreenrecorder-2023-12-22_13.18.15.mp4

I'm not sure where to put the documentation for this feature; honestly, I would rather wait with it until custom units are supported in Bokeh.

@hoxbro hoxbro marked this pull request as ready for review December 22, 2023 12:24
@droumis
Copy link
Member

droumis commented Jan 4, 2024

Linking to the Bokeh PR for custom units

@droumis droumis mentioned this pull request Feb 20, 2024
16 tasks
@droumis
Copy link
Member

droumis commented Apr 5, 2024

What is left to do for this PR?

@hoxbro
Copy link
Member Author

hoxbro commented Apr 5, 2024

Implement the custom unit introduced in bokeh/bokeh#13625

@droumis
Copy link
Member

droumis commented Apr 5, 2024

Just to connect this PR with the original use-case requirements spec: holoviz-topics/neuro#34

@droumis droumis added this to the 1.19.0 milestone May 28, 2024
@hoxbro
Copy link
Member Author

hoxbro commented May 28, 2024

I have added a tool to hide scale bars.

import holoviews as hv
import numpy as np

hv.extension("bokeh")

c1 = hv.Curve(np.random.rand(1000) + 2, label="c1").opts(
    scalebar=True, scalebar_location="bottom_left", scalebar_unit="cm"
)
c2 = hv.Curve(np.random.rand(1000), label="c2").opts(scalebar=True, scalebar_location="top_left")

curves = hv.Overlay(c1 * c2)
curves.opts(width=800, height=800)
screenrecord-2024-05-28_19.37.06.mp4

@droumis droumis self-requested a review May 29, 2024 14:19
@droumis
Copy link
Member

droumis commented May 30, 2024

What's with the html? I see other reference notebooks just using markdown

<div class="contentcontainer med left" style="margin-left: -50px;">
<dl class="dl-horizontal">
  <dt>Title</dt> <dd> Scalebar</dd>
  <dt>Dependencies</dt> <dd>Bokeh</dd>
  <dt>Backends</dt> 
    <dd><a href='./Scalebar.ipynb'>Bokeh</a></dd>
</dl>
</div>

@droumis
Copy link
Member

droumis commented May 30, 2024

I don't quite understand how to use the kdims to apply an inferred unit.

From the description ('The unit of the scalebar tries to infer it for the first kdims.'), it sounds like this should change the unit to Voltage:

image

And this to Time:

Code
from datetime import datetime, timedelta

times = [datetime.now() + timedelta(seconds=i) for i in range(1000)]
hv.Curve((times, np.random.rand(1000)), kdims=['Time']).opts(scalebar=True)
image

@droumis
Copy link
Member

droumis commented May 30, 2024

hmm.. ok I kind of see how to set it via hv.dim

image

But I guess there's no way to set both the base-dimension and the dimension with hv.Dimension. It might be a pretty narrow use case where the 'inferred' (semi-explicit but partial via hv.Dimension) approach would be sufficient. I think we should consider moving the inferred approach to a future PR, unless I'm really missing something.

@hoxbro
Copy link
Member Author

hoxbro commented May 30, 2024

What's with the html? I see other reference notebooks just using markdown

Honest to god, I just clicked on a random reference guide, and that was how it was done in that 🤷‍♂️

hmm.. ok I kind of see how to set it via hv.dim

You are right. That is the way to do it. Just if you are unaware, hv.dim is something else than hv.Dimension.

Maybe we should update the Example to show how to use hv.Dimension.

@droumis
Copy link
Member

droumis commented Jun 3, 2024

I only see success when using a fully specified scalebar_unit arg, so I will only document that approach until future PRs address other approaches.

If it is going to take a while to address the identified issues, I think we should consider delaying other methods for specifying the units until future PRs (including only supplying a single string to scalebar_unit), so we can proceed with the release.

image

Unsuccessful approaches

image image image image

@droumis
Copy link
Member

droumis commented Jun 3, 2024

I also don't yet see support for a scalebar_icon arg so I'll remove this from the documentation for now

@hoxbro
Copy link
Member Author

hoxbro commented Jun 3, 2024

I also don't yet see support for a scalebar_icon arg so I'll remove this from the documentation for now

Was an earlier name for scalebar_tool 🙃

@droumis
Copy link
Member

droumis commented Jun 3, 2024

OK I think the docs are ready.

image

@hoxbro hoxbro requested a review from philippjfr June 5, 2024 09:02
scalebar = param.Boolean(default=False, doc="""
Whether to display a scalebar.""")

scalebar_range =param.Selector(default="x", objects=["x", "y"], doc="""
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
scalebar_range =param.Selector(default="x", objects=["x", "y"], doc="""
scalebar_range = param.Selector(default="x", objects=["x", "y"], doc="""

Copy link
Member

@philippjfr philippjfr left a comment

Choose a reason for hiding this comment

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

I'm okay with merging this but ideally we'd handle updates to the scalebar options.

@philippjfr philippjfr merged commit dd0e4d5 into main Jun 6, 2024
14 checks passed
@philippjfr philippjfr deleted the scalebar branch June 6, 2024 12:27
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 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
czi type: feature A major new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ScaleBar support
4 participants