images from ROIs all ZT planes

[will-moore]

See https://trello.com/c/my7BGCEb/7-crop-iviewer-and-images-from-rois

This supports cropping a multi-Z or multi-T image to create a new image with the same number of Z or T planes. This was supported before, but needed a multi-shape ROI with shapes on all the Z/T planes to be included. Now, if an ROI has no Z set on its shape(s) then we use all Z planes. Same for T. To test:

• Using an image with multiple Z and T, create several rectangles, some with Z and T set, some with no Z or T set. See all combinations in screenshot.
• Run the Images from ROIs on the image.
• If a shape has no Z set, we create new image with all Z planes, same for T.
• For screenshot below, we created 1 single-plane image, 1 single-Z timelapse, 1 single-T z-stack and 1 image of same Z and T size as original.

[jburel]

travis failure: see https://github.com/ome/scripts/pull/136#issuecomment-351191004

We could include the change in the devspace and ignore travis failure for now but that might put us in a precarious position

[joshmoore]

Alternatively, we say it's time to open next_infra against develop.

[jburel]

@joshmoore probably

[joshmoore]

Since it's just the one PR and it's a fast-forward, I've pushed next_infra onto develop:

$ git show-branch origin/{develop,next_infra,workflows}
! [origin/develop] Merge pull request #135 from will-moore/label_rotate_fix
 ! [origin/next_infra] Merge pull request #136 from jburel/test-in-travis
  ! [origin/workflows] Merge pull request #135 from will-moore/label_rotate_fix
---
 -  [origin/next_infra] Merge pull request #136 from jburel/test-in-travis
 +  [origin/next_infra^2] Update name of test-omero
 +  [origin/next_infra^2^] Update to use test-omero
 +  [origin/next_infra^2~2] Parse inputs
 +  [origin/next_infra^2~3] Flake8 scripts
 +  [origin/next_infra^2~4] Fix typo
 +  [origin/next_infra^2~5] Add doc
 +  [origin/next_infra^2~6] Add line
 +  [origin/next_infra^2~7] Mark the movie test as fail
 +  [origin/next_infra^2~8] Enable check
 +  [origin/next_infra^2~9] Minor edit
 +  [origin/next_infra^2~10] Unify extension
 +  [origin/next_infra^2~11] Print error
 +  [origin/next_infra^2~12] Organize check
 +  [origin/next_infra^2~13] Change range
 +  [origin/next_infra^2~14] Fix script
 +  [origin/next_infra^2~15] Change image dimension
 +  [origin/next_infra^2~16] no longer install pillow and numpy via pip
 +  [origin/next_infra^2~17] Add export test
 +  [origin/next_infra^2~18] Add movie dependency
 +  [origin/next_infra^2~19] Add test
 +  [origin/next_infra^2~20] Install dependencies
 +  [origin/next_infra^2~21] Add more tests
 +  [origin/next_infra^2~22] Add extra line
 +  [origin/next_infra^2~23] Add new support for more versions
 +  [origin/next_infra^2~24] Exclude .DS_Store
 +  [origin/next_infra^2~25] Add support for multiple versions
 +  [origin/next_infra^2~26] adjustment
 +  [origin/next_infra^2~27] Review setup
 +  [origin/next_infra^2~28] Run pytest
 +  [origin/next_infra^2~29] Change permissions
 +  [origin/next_infra^2~30] Do not check readme
 +  [origin/next_infra^2~31] Run scripts in travis
--- [origin/develop] Merge pull request #135 from will-moore/label_rotate_fix

Closing and re-opening to launch travis.

[joshmoore]

test/integration/test_util_scripts.py::TestUtilScripts::test_images_from_rois FAILED [ 81%]
...
  File "/opt/omero/server/OMERO.server/lib/python/omero/gateway/__init__.py", line 3750, in createImageFromNumpySeq

    raise exc

ValueError: invalid value for argument 1 in operation `getTile'

[joshmoore]

@will-moore : did you want to try to fix this?

[will-moore]

@joshmoore That should fix the test.

[joshmoore]

./omero/util_scripts/Images_From_ROIs.py:234:17: N806 variable 'theZ' in function should be lowercase
./omero/util_scripts/Images_From_ROIs.py:235:17: N806 variable 'theT' in function should be lowercase

[will-moore]

@joshmoore How do I run these tests locally? I get some strange result with

$ ./setup.py test -t test/integration/test_util_scripts.py 
Version: ImageMagick 6.9.3-6 Q16 x86_64 2016-02-28 http://www.imagemagick.org
Copyright: Copyright (C) 1999-2016 ImageMagick Studio LLC
License: http://www.imagemagick.org/script/license.php
Features: Cipher DPC Modules 
Delegates (built-in): bzlib freetype jng jpeg ltdl lzma png tiff xml zlib
Usage: import [options ...] [ file ]

I cloned...

git clone --recurse-submodules git://github.com/openmicroscopy/omero-test-infra .omero

But I don't see anything obvious there. Do I have to use docker to run tests?

[joshmoore]

From https://github.com/ome/scripts/blob/develop/.travis.yml#L15:

.omero/scripts-docker

[will-moore]

.omero/scripts-docker seems to do a lot of installing and flake8 checking (and failing) but I'm not seeing any tests run:

.omero/scripts-docker
...
./omero/export_scripts/Export_to_other_omero.py:157:9: E116 unexpected indentation (comment)
./omero/export_scripts/Export_to_other_omero.py:173:22: W292 no newline at end of file
flake8.main.application   MainProcess   1873 INFO     Found a total of 359 violations and reported 161

[joshmoore]

You mean locally? On travis, I see this:

test/integration/test_util_scripts.py::TestUtilScripts::test_images_from_rois[True] PASSED [ 78%]
test/integration/test_util_scripts.py::TestUtilScripts::test_images_from_rois[False] PASSED [ 82%]

under https://travis-ci.org/ome/scripts/builds/329477205#L671

i.e. :+1:

[will-moore]

Yes, I want to be able to run the tests locally, 1 test at a time, while writing & debugging (before committing).

[joshmoore]

Hmmm.... a couple of things:

• Mea culpa but I certainly was thinking more about the travis use-case than the developer use case. Happy for us to look into adding dev support, but it might take some back and forth.
• Likely the first goal of that would be to add an argument or environment variable to choose a single test, which doesn't sound too hard.
• It looks like though that you're not making it to the tests locally since flake8 is failing. Perhaps this is due to the state of your branch?? (Travis merges the branch into the mainline which may fix existing flake8 issues)
• If that still doesn't help, then we could look into making the flaking optional, but that shouldn't be a first resort.

[will-moore]

Not sure if "mainline" is develop or test_infra, so I tried both, but no joy...

$ git fetch origin
$ git merge origin/develop
Already up-to-date.
$ git merge origin/next_infra
Already up-to-date.
$ .omero/scripts-docker
...
./omero/export_scripts/Export_to_other_omero.py:173:22: W292 no newline at end of file

Now I don't see the summary message from above

flake8.main.application   MainProcess   1873 INFO     Found a total of 359 violations and reported 161

but otherwise looks the same.

[joshmoore]

Can you try fixing that file and/or commenting out flake8 under .omero/py-setup?

[will-moore]

Ah, sorry - I've got a couple of untracked local files that are getting included. I'll move them...

[will-moore]

Passing now: ==================== 27 passed, 1 xfailed in 180.86 seconds ====================

[joshmoore]

Nice!

[jburel]

Not for that PR but we should add to the description of the new image the ID of the source image. This is added only when the "stack option is on"

The stack option will need to be reviewed since we end up with a one channel image not really consistent with the rest of the script. I will create card for the various points

The changes made in this PR works as expected. The PR re-introduced some print statement we should agree on the approach, useful for debugging but can be annoying in prod

Ready for 5.4.2

[will-moore]

Print statements are a simple way of logging to the stdout file that is shown to users at the "Info" button and we use them in all our scripts. The Make Stack option was the original behaviour of this script since it applies to cryo-EM workflows where you typically compile many similar-shaped Rect ROIs on a single-plane image into a Z-stack. We could add more explanation in the description, but it's already quite lengthy. Currently it includes "If you choose to 'make an image stack' from all the ROIs, the script will create a single new Z-stack image with a single plane from each ROI."

[joshmoore]

... useful for debugging but can be annoying in prod

Had never considered it before, but it'd be fairly straight-forward to have omero.scripts.client setting a debugging level so that all scripts could begin using logging and have the clients allow users to choose their verbosity.

[joshmoore]

@jburel will open a card for the remaining issues. Merging for inclusion in 5.4.2