-
Notifications
You must be signed in to change notification settings - Fork 32
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
images from ROIs all ZT planes #137
images from ROIs all ZT planes #137
Conversation
travis failure: see #136 (comment) We could include the change in the devspace and ignore travis failure for now |
Alternatively, we say it's time to open |
@joshmoore probably |
Since it's just the one PR and it's a fast-forward, I've pushed next_infra onto develop:
Closing and re-opening to launch travis. |
|
@will-moore : did you want to try to fix this? |
@joshmoore That should fix the test. |
In testing ome/omero-scripts#137 this was originally passing with Make_Image_Stack was False but failing if True. Now we test with both and check the created image has the expected sizeZ
|
this was originally passing with Make_Image_Stack was False but failing if True. Now we test with both and check the created image has the expected sizeZ
@joshmoore How do I run these tests locally?
I cloned...
But I don't see anything obvious there. Do I have to use docker to run tests? |
From https://github.com/ome/scripts/blob/develop/.travis.yml#L15:
|
|
You mean locally? On travis, I see this:
under https://travis-ci.org/ome/scripts/builds/329477205#L671 i.e. 👍 |
Yes, I want to be able to run the tests locally, 1 test at a time, while writing & debugging (before committing). |
Hmmm.... a couple of things:
|
Not sure if "mainline" is develop or test_infra, so I tried both, but no joy...
Now I don't see the summary message from above
but otherwise looks the same. |
Can you try fixing that file and/or commenting out flake8 under |
Ah, sorry - I've got a couple of untracked local files that are getting included. I'll move them... |
Passing now: |
Nice! |
Not for that PR but we should add to the description of the new image The stack option will need to be reviewed since we end up with a one channel image The changes made in this PR works as expected. The PR re-introduced some print statement Ready for 5.4.2 |
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. |
Had never considered it before, but it'd be fairly straight-forward to have |
@jburel will open a card for the remaining issues. Merging for inclusion in 5.4.2 |
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: