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

reverseIntensity keyError export #229

Merged
merged 1 commit into from Jul 3, 2017

Conversation

Projects
None yet
4 participants
@will-moore
Member

will-moore commented Jun 29, 2017

Fixes bug with export of images that were added to figures before 'reverseIntensity' was supported (before 5.3.0).
Reported to @kennethgillen from nightshade:
https://trello.com/c/baovPrAJ/7-reverseintensity-keyerror-in-export

To test:

  • I reproduced the bug by manually removing 'reverseIntensity' info from the JSON:
    • File -> Export as JSON
    • Copy this data, removing the , "reverseIntensity": true (only need to do this for a single active channel of a single image.
    • Create a new figure with this text via File -> Import from JSON.
  • Export of this figure should not cause any error.

cc @jburel - We should probably release this as a 3.1.1 bug-fix ASAP.

@@ -759,7 +759,7 @@ def apply_rdefs(self, image, channels):
c_idxs.append(i+1)
windows.append([c['window']['start'], c['window']['end']])
colors.append(c['color'])
reverses.append(c['reverseIntensity'])
reverses.append(c.get('reverseIntensity', False))

This comment has been minimized.

@sbesson

sbesson Jun 29, 2017

Member

Generally 👍 for generalizing the use of get(value, default) for an optional attribute.

Beyond the scope of this fix, as we might be introducing more non-breaking functionalities like reverseIntensity, we might have to think on how best to codify this knowledge using some formal schema/specification so that others can consume the OMERO.figure format/API and know what to expect.

@sbesson

sbesson Jun 29, 2017

Member

Generally 👍 for generalizing the use of get(value, default) for an optional attribute.

Beyond the scope of this fix, as we might be introducing more non-breaking functionalities like reverseIntensity, we might have to think on how best to codify this knowledge using some formal schema/specification so that others can consume the OMERO.figure format/API and know what to expect.

This comment has been minimized.

@jburel

jburel Jul 3, 2017

Member

we will also need old testing data in our setup.
I have created https://trello.com/c/h9WOaIkv/10-produce-older-figures-in-the-testing-db
unless we run nightshade clone somewhere later on on

@jburel

jburel Jul 3, 2017

Member

we will also need old testing data in our setup.
I have created https://trello.com/c/h9WOaIkv/10-produce-older-figures-in-the-testing-db
unless we run nightshade clone somewhere later on on

@pwalczysko

This comment has been minimized.

Show comment
Hide comment
@pwalczysko

pwalczysko Jun 30, 2017

Member

Tested both via editing the json as well as finiding some old figures on eel and verifying that both the edited json as well as the old figure (pre-5.3.0) exports fine. The crash is repeatable on nightshade as well with old figures.

All works as expected. Ready to merge.

Member

pwalczysko commented Jun 30, 2017

Tested both via editing the json as well as finiding some old figures on eel and verifying that both the edited json as well as the old figure (pre-5.3.0) exports fine. The crash is repeatable on nightshade as well with old figures.

All works as expected. Ready to merge.

@jburel jburel merged commit 1909620 into ome:master Jul 3, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jburel jburel modified the milestone: 3.1.1 Jul 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment