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

Ignore hidden scalebars #381

Merged
merged 1 commit into from
May 11, 2020
Merged

Conversation

will-moore
Copy link
Member

This fixes a minor bug, brought to light by an error from user at #379.
The error was a KeyError because 'length' was missing from the scalebar dict (don't know why). The error came from the "Info-page" code, where we add scalebars to the the info page, and I noticed that this code was not taking the visibility of the scalebar into account.

Now, we ignore the hidden scalebars on the info page AND access length with ```scalebar.get('length') to avoid KeyError.
Fixes #379

To test:

  • Add scalebars of different lengths to 2 different panels
  • Hide 1 of the scalebars and export the PDF
  • The hidden scalebar should be ignored in the "Scalebar lenghts" of the page-info

when showing lengths of scalebars in the info page
@will-moore will-moore changed the title Only use visible scalebars in figure-info Ignore hidden scalebars May 10, 2020
@jburel jburel mentioned this pull request May 11, 2020
@jburel
Copy link
Member

jburel commented May 11, 2020

The scale bar is no longer in the info page.
Now the confusion starts, which scale corresponds to which image?
When all the scale bars are visible, scale order = image order in Figure contains the following images.
Now user will not know with one corresponds to which image

@will-moore
Copy link
Member Author

That's kinda outwith the scope of this PR.
If you can think of a nice way to indicate that, then it might be worth adding, but I think it's quite an edge-case.

Usually you only have a single size of scalebar in a figure and the figure legend will say "Scale bars are 10 microns". If you have multiple different sizes of scalebar users tend to add a label to each scalebar so this info is is not really needed in the figure legend, but it doesn't do any harm.

@jburel
Copy link
Member

jburel commented May 11, 2020

One option could be to have the scale bar length after the link of the image.

I don't think this is outside this PR, since now the user will be confused since one is hidden and have a different scale bar

@will-moore
Copy link
Member Author

This PR fixes a Bug: if I have 2 images, only 1 of them shows a scalebar of 10um (the other has scalebar 20 um but the scalebar is hidden), but the info-page says "Scalebar Lengths 10 um, 20 um". This is fixed so the info-page ignores the hidden scalebar and correctly says "Scalebar Lengths 10 um". That is less confusing than it was before.

@jburel
Copy link
Member

jburel commented May 11, 2020

but the user does not know which one is the scale bar

@jburel
Copy link
Member

jburel commented May 11, 2020

Discussed privately with @will-moore, changing the current layout of the PDF will be challenging.
We have more urgent things to do and balancing the time vs feature probably not worth it.
I am still not convinced of the need of the Scale bar feature in the PDF when we have the scale bars on the images (if not hidden)

This PR fixes the problem in the current implementation

@jburel jburel merged commit d246d80 into ome:master May 11, 2020
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.

Bug: Scalebar length KeyError in export script
2 participants