Skip to content

Version detection fix#44

Merged
jburel merged 4 commits intoome:masterfrom
sbesson:version_fix
Sep 7, 2020
Merged

Version detection fix#44
jburel merged 4 commits intoome:masterfrom
sbesson:version_fix

Conversation

@sbesson
Copy link
Member

@sbesson sbesson commented Sep 6, 2020

Reported by @jburel. In the absence of an explicit version key in the rendering file, the detection logic in _getversion() is currently checking the presence of start/end vs min/max keys (as the semantics of these has changed since version 1 and 2) and unconditionally returning 0 (undefined version) in absence of any of these keys. This means a simple rendering file as the following is currently detected as invalid:

---
channels:
  1:
    label: 'DNA' 

ab081ac fixes this issue by returning the version of the latest specification version (2)in this case. 28a10df also increases the type and the range of supported versions when the version argument is present. Several unit tests have been added that should test various YML files and the plugin expectation when loading these.

Travis unit and integration tests should keep passing. Additionally, a simple test should be run with a minimal YML file like the one mentioned above. omero render set should fail without this PR and pass with this PR included.

Proposed relase: 0.6.1

@jburel
Copy link
Member

jburel commented Sep 7, 2020

Tested the culprit renderer.yml, no problem noticed this time:

  • no version specified
channels:
  1:
    # gray
    color: "808080"
  • version specified
version: 2
channels:
  1:
    # gray
    color: "808080"

and

  • version specified
version: 1
channels:
  1:
    # gray
    color: "808080"

@sbesson
Copy link
Member Author

sbesson commented Sep 7, 2020

i.e. LGTM and released?

@jburel
Copy link
Member

jburel commented Sep 7, 2020

Sorry
I wrote another comment but i did not press the button 👍
merging

@jburel jburel merged commit da44a72 into ome:master Sep 7, 2020
@sbesson sbesson deleted the version_fix branch September 7, 2020 11:59
@sbesson
Copy link
Member Author

sbesson commented Sep 7, 2020

Now released as https://pypi.org/project/omero-cli-render/0.6.1/

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.

2 participants