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

render set: helper refactoring #34

Merged
merged 10 commits into from
Nov 29, 2019
Merged

render set: helper refactoring #34

merged 10 commits into from
Nov 29, 2019

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented Nov 28, 2019

While working on a feature allowing to process multiple images with multiple rendering settings, I found it useful to refactor the logic of the render set command dealing with loading and parsing rendering settings from a source (object or file on disk).

The changes included in this PR should not changing behavior, include some cleanup (print ->debug) as well as unit tests starting to cover some of the individual functionalities

@snoopycrimecop
Copy link
Member

snoopycrimecop commented Nov 29, 2019

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

--conflicts Conflict resolved in build OMERO-plugins-push#110. See the console output for more details.

src/omero_cli_render.py Outdated Show resolved Hide resolved
@dominikl
Copy link
Member

dominikl commented Nov 29, 2019

Looks good. Just one comment (see above), and when I ran it, got a warning:

/home/dominik/py3/lib/python3.6/site-packages/omero/util/pydict_text_io.py:81: YAMLLoadWarning: calling yaml.load_all() without Loader=... is deprecated, as the default Loader is unsafe. Please read https://msg.pyyaml.org/load for full details.
  data = list(yaml.load_all(rawdata))

Edit: The warning doesn't have anything to do with the PR though, seems to be a python3 thing.

@dominikl
Copy link
Member

Tested again, works. Good to merge 👍

@sbesson sbesson merged commit 957bb12 into ome:master Nov 29, 2019
@sbesson sbesson deleted the set_helpers branch November 29, 2019 15:41
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.

3 participants