Skip to content

Preview rdef start end ints#5484

Merged
jburel merged 6 commits intoome:developfrom
will-moore:preview_rdef_start_end_ints
Sep 25, 2017
Merged

Preview rdef start end ints#5484
jburel merged 6 commits intoome:developfrom
will-moore:preview_rdef_start_end_ints

Conversation

@will-moore
Copy link
Copy Markdown
Member

What this PR does

Fixes an error loading Preview panel when the rendering setting channel start/end values are floats, not integers. Although we only see this error on web-dev-integration (doesn't fail locally, maybe different Python version).

Internal Server Error: /webclient/metadata_preview/image/5595/
Traceback (most recent call last):
...
 File "/opt/hudson/workspace/WEB-DEV-integration-deploy/OMERO.py/lib/python/omeroweb/webclient/views.py", line 1585, in load_metadata_preview
   % (act, i+1, c['start'], c['end'], reverse, color))
TypeError: %d format: a number is required, not float

However, this also highlighted that we shouldn't be casting start/end to Integer because some float images have very small ranges and it's important to use float start/end values.
Fixed another example of casting these to Integer in the JavaScript viewer.

Testing this PR

  1. Find/import a floating point image with small dynamic range, e.g. /test_images_good/EM-data/EMD-2225.map
  2. Adjust rendering settings, save them.
  3. Re-load the Preview panel, adjust settings again then click the thumbnail showing your rendering settings to re-load the saved settings.
  4. These actions should all support floating values for start/end.

@jburel jburel added the develop label Sep 1, 2017
@snoopycrimecop
Copy link
Copy Markdown
Member

Conflicting PR. Removed from build OMERO-DEV-merge-push#790. See the console output for more details.
Possible conflicts:

  • PR Api experimenters groups #5315 will-moore 'Api experimenters groups'
    • components/tools/OmeroWeb/omeroweb/webclient/views.py
  • PR Reverse invert webclient #5470 will-moore 'Reverse invert webclient'
    • components/tools/OmeroWeb/omeroweb/webclient/views.py
  • Upstream changes
    • components/tools/OmeroWeb/omeroweb/webclient/views.py

@snoopycrimecop
Copy link
Copy Markdown
Member

Conflicting PR. Removed from build OMERO-DEV-merge-push#791. See the console output for more details.
Possible conflicts:

  • PR Api experimenters groups #5315 will-moore 'Api experimenters groups'
    • components/tools/OmeroWeb/omeroweb/webclient/views.py
  • PR Reverse invert webclient #5470 will-moore 'Reverse invert webclient'
    • components/tools/OmeroWeb/omeroweb/webclient/views.py
  • Upstream changes
    • components/tools/OmeroWeb/omeroweb/webclient/views.py

@snoopycrimecop
Copy link
Copy Markdown
Member

Conflicting PR. Removed from build OMERO-DEV-merge-push#792. See the console output for more details.
Possible conflicts:

  • PR Reverse invert webclient #5470 will-moore 'Reverse invert webclient'
    • components/tools/OmeroWeb/omeroweb/webclient/views.py
  • Upstream changes
    • components/tools/OmeroWeb/omeroweb/webclient/views.py

@snoopycrimecop
Copy link
Copy Markdown
Member

Conflicting PR. Removed from build OMERO-DEV-merge-push#793. See the console output for more details.
Possible conflicts:

  • PR Reverse invert webclient #5470 will-moore 'Reverse invert webclient'
    • components/tools/OmeroWeb/omeroweb/webclient/views.py
  • Upstream changes
    • components/tools/OmeroWeb/omeroweb/webclient/views.py

@snoopycrimecop
Copy link
Copy Markdown
Member

Conflicting PR. Removed from build OMERO-DEV-merge-push#794. See the console output for more details.
Possible conflicts:

  • PR Reverse invert webclient #5470 will-moore 'Reverse invert webclient'
    • components/tools/OmeroWeb/omeroweb/webclient/views.py
  • Upstream changes
    • components/tools/OmeroWeb/omeroweb/webclient/views.py

@snoopycrimecop
Copy link
Copy Markdown
Member

Conflicting PR. Removed from build OMERO-DEV-merge-push#796. See the console output for more details.
Possible conflicts:

  • PR Reverse invert webclient #5470 will-moore 'Reverse invert webclient'
    • components/tools/OmeroWeb/omeroweb/webclient/views.py
  • Upstream changes
    • components/tools/OmeroWeb/omeroweb/webclient/views.py

@snoopycrimecop
Copy link
Copy Markdown
Member

Conflicting PR. Removed from build OMERO-DEV-merge-push#797. See the console output for more details.
Possible conflicts:

  • Upstream changes
    • components/tools/OmeroWeb/omeroweb/webclient/views.py

@will-moore
Copy link
Copy Markdown
Member Author

@rgozim Did you get a chance to look at this?

@rgozim
Copy link
Copy Markdown
Member

rgozim commented Sep 14, 2017

Tested on web-dev-merge. Works as described.

@jburel
Copy link
Copy Markdown
Member

jburel commented Sep 14, 2017

Could a robot test be doable in that case?
i.e. import a float image and open the preview panel

@will-moore
Copy link
Copy Markdown
Member Author

Updated robot test to check that rdef settings (floats) are copied, pasted & applied correctly.

@jburel
Copy link
Copy Markdown
Member

jburel commented Sep 18, 2017

I think you should keep a mix of int and floats not all floats in the tests

@will-moore
Copy link
Copy Markdown
Member Author

Why?

@will-moore
Copy link
Copy Markdown
Member Author

They are both testing different actions (Copy -> 'Paste & Save' and click rdef thumbnail)

@jburel
Copy link
Copy Markdown
Member

jburel commented Sep 19, 2017

To be sure handle both correctly

@will-moore
Copy link
Copy Markdown
Member Author

Added testing of copy & paste of start/end Integers into another test.

@jburel
Copy link
Copy Markdown
Member

jburel commented Sep 20, 2017

@will-moore thanks

@dominikl
Copy link
Copy Markdown
Member

dominikl commented Sep 21, 2017

PR works as described, the Test Rdef Copy Paste Save test passed, too. But Test Owners Rdef failed today. Can't really rule out that this is not related to this PR. Rerun the robot tests maybe?

@dominikl
Copy link
Copy Markdown
Member

dominikl commented Sep 21, 2017

Argh, that was yesterday's robot tests, today's not finished yet. Will check again later.

@dominikl
Copy link
Copy Markdown
Member

Finally dev-merge-robot job ran again. There are a couple of Rdef related failed tests. I guess that might have been caused by this PR. https://ci.openmicroscopy.org/view/Failing/job/OMERO-DEV-merge-robotframework/718/robot/

@will-moore
Copy link
Copy Markdown
Member Author

Hmmm - All Rdef tests are passing for me locally on this branch... Let's see what test result we get today...

@will-moore
Copy link
Copy Markdown
Member Author

Rdef tests all passing today: https://ci.openmicroscopy.org/view/Failing/job/OMERO-DEV-merge-robotframework/719/robot/

@dominikl
Copy link
Copy Markdown
Member

Good to merge then. Manual testing of the PR worked as described. 👍

@jburel jburel merged commit aee1d59 into ome:develop Sep 25, 2017
@sbesson sbesson added this to the 5.4.0 milestone Oct 6, 2017
@will-moore will-moore deleted the preview_rdef_start_end_ints branch February 18, 2019 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants