-
Notifications
You must be signed in to change notification settings - Fork 239
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
FakeReader improvements, part 1 #1191
Conversation
Prior to this, multi-file screens could only be detected from the top-level directory. This meant that importing into OMERO would not have the desired effect, as ImportCandidates automatically recurses into the directory and picks up the individual field files. Fixes #12430.
See #12430. It's a little weird to have e.g. 'mkfake ~/' create ~/.fake/, and makes it a little more difficult to find or remove the screen directory. 'mkfake ~/' with this change will now create ~/screen.fake/, or ~/screenX.fake/ where X is the smallest index (from 1) for which a directory does not exist.
The 'color_x' keys (where x is the channel index) now set the color for the specified channel. These keys can be used with 'color', which acts as a default value for any missing or invalid 'color_x' values. Fixes #12423.
mkfake has changed so that it now generated a screen.fake, but the import behaviour seems unchanged I'm afraid.
|
One more commit added to fix import - this was just due to the used file list being incorrect. |
NB: @sbesson, for your upcoming import test PR, worth adding a |
Travis connection failure; restarted |
Absolutely. Will add to ome/openmicroscopy#2736 if no objection. |
Makes sense to me. |
Great, it is now working when importing screens with multiple plates. Couple of small things however. Is it intended that if the screen does not have multiple plates then it will not be recognised as a screen? If so, then I think the default behaviour of mkfake should be altered slightly to instead make the simplest thing that does qualify as a screen:
This actually happens with any combination of Cheers |
The other thing is that even if the screen imports correctly, if you run
|
Actually, here is an objection: #2736 is opened against |
Fake screen import unit test request captured in https://trello.com/c/JO7d1udy |
Add HCS domain when fake image is SPW.
Merged PR from @bpindelski to address SPW detection, and added one more commit to address test failures. |
Files are being read properly (SPW detection works),
but import fails with the following exception (Identifying .DS_Store file and not ignoring it)
|
Import failure of All good to merge otherwise? |
Independent testing by ome/openmicroscopy#2782 confirms this PR is doing what is expected. |
Based on this output, I would assume the
|
yup it is showing in the used file list. So its bound to get imported right. |
Sorry - missed that bit in the output. One more commit added to clean up the used file list. |
Everything works as expected. Please merge.
|
FakeReader improvements, part 1
--rebased-to #1217 |
Fixes https://trac.openmicroscopy.org.uk/ome/ticket/12430 and https://trac.openmicroscopy.org.uk/ome/ticket/12423.
For 12430, running the
mkfake
test case from the ticket and importing into OMERO should result in a single Screen with 2 Plates being created, instead of all Images being unlinked and in datasets.For 12423, specifying channel colors via the
color_x
keys (wherex
is the channel index) should allow each channel color to be set individually. The colors shown in the OME-XML or upon import into OMERO should match the values set by thecolor_x
keys./cc @dpwrussell, @bpindelski