-
Notifications
You must be signed in to change notification settings - Fork 10
Support export of Fileset:ID #123
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
base: master
Are you sure you want to change the base?
Support export of Fileset:ID #123
Conversation
joshmoore
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the code & the description:
(would need different
objectparameter type since currentProxyStringTypedoesn't support multiple IDs).
Could try to add support, though that might be ProxiesStringType :)
I guess we might want to expose this in the UI a bit better?
👍 Considering there are CLI operations on it, that'd be my vote.
| return transformations | ||
|
|
||
|
|
||
| def get_minimum_image_ome_xml(images: List[ImageWrapper]) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth considering ome_model or ome_types for this? cc: @seb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using ome-types in 553b35f
|
Webclient PR to show Fileset ID: ome/omero-web#385 |
pwalczysko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address ome/omero-web#385 (comment)
pwalczysko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, igoner my review, it was not on the correct PR.#123 (review)
|
@pwalczysko, but do you also want to give this PR a try? 😉 |
|
|
I couldn't reproduce that error. But in either case I didn't see the error above. @pwalczysko can you reproduce on a clean conda env? |
I think that the discrepancy is in the python version, my envs have python 3.6. Will try |
|
Running a gives me a following error @will-moore Details |
|
Issue created at #127 |
|
@pwalczysko I don't see |
elicits the error below @will-moore (note python version in the above command) Details |
|
Ah - OK, thanks. So the use of python-3.6 should also be handled by better install instructions: #127 If you install |
Yes, this is what I did and it works. |
|
Is this functionality supposed to produce real ome.zarr images with pixeldata ? See https://merge-ci.openmicroscopy.org/web/webclient/?show=image-243571 - user-3, after reimport into OMERO, all values reported as 0, the original image is the https://merge-ci.openmicroscopy.org/web/webclient/?show=image-200515 The same is repeating with any image I tried up till now. |
|
@pwalczysko yes, it is supposed to! It looks like the data chunks haven't been uploaded. Although they appear OK in Trying to export and re-import a single image: Trying to import a single image from the Fileset exported above... Seems that Bio-Formats doesn't recognise this as an OME-NGFF image without To summarise, import either fails to import the chunks (and you end up with all pixel values of 0) or if chunks are uploaded then the import fails for another reason. cc @dgault any ideas what's going on here? |
|
Thank you @will-moore for the explanation and follow-up testing. From my side, the functionality in this PR works as described, but the reimport of the ome.zarr into OMERO fails as detailed in #123 (comment) - I think this PR is okay, but maybe we should wait with the merging to find out what must be done to fix the complete export -> reimport workflow ? |
|
Trying to import the |
@will-moore @dgault further to #123 (comment) - I do not think that the correct reader is used, see above. |
(Side note: internally and externally we should always start from |
|
This pull request has been mentioned on Image.sc Forum. There might be relevant details there: https://forum.image.sc/t/export-large-images-from-omero-via-web/84652/3 |
|
Conflicting PR. Removed from build OMERO-plugins-push#343. See the console output for more details.
--conflicts |
|
Conflicting PR. Removed from build OMERO-plugins-push#344. See the console output for more details.
--conflicts |
|
Conflicting PR. Removed from build OMERO-plugins-push#345. See the console output for more details.
--conflicts |
|
Conflicting PR. Removed from build OMERO-plugins-push#346. See the console output for more details.
--conflicts |
|
Conflicting PR. Removed from build OMERO-plugins-push#347. See the console output for more details.
--conflicts |
|
Conflicting PR. Removed from build OMERO-plugins-push#348. See the console output for more details.
--conflicts |
|
Conflicting PR. Removed from build OMERO-plugins-push#349. See the console output for more details.
--conflicts |
|
Conflicting PR. Removed from build OMERO-plugins-push#350. See the console output for more details.
--conflicts |
|
Conflicting PR. Removed from build OMERO-plugins-push#351. See the console output for more details.
--conflicts |
|
Conflicting PR. Removed from build OMERO-plugins-push#352. See the console output for more details.
--conflicts |
|
Conflicting PR. Removed from build OMERO-plugins-push#353. See the console output for more details.
--conflicts |
|
Conflicting PR. Removed from build OMERO-plugins-push#354. See the console output for more details.
--conflicts |
|
Conflicting PR. Removed from build OMERO-plugins-push#355. See the console output for more details.
--conflicts |
|
Conflicting PR. Removed from build OMERO-plugins-push#356. See the console output for more details.
--conflicts |
|
Conflicting PR. Removed from build OMERO-plugins-push#357. See the console output for more details.
--conflicts |
|
Conflicting PR. Removed from build OMERO-plugins-push#358. See the console output for more details.
--conflicts |
|
Conflicting PR. Removed from build OMERO-plugins-push#359. See the console output for more details.
--conflicts |
|
NB: to possibly fix the failure to re-import above (no chunks etc) we need to remove the |
|
--exclude |
Fixes #122
To test: e.g. on merge-ci (user-3):
Will create a
test_filesetdirectory with images/0,/1etc in it.Directory will contain
.zattrsand/OME/METADATA.ome.xmlas defined in ome/ngff#112If directory exists, then
test_fileset(1)directory will be created etc.Support for other multi-image export to the same format NOT included yet. Could come in a follow-up PR e.g:
(would need different
objectparameter type since currentProxyStringTypedoesn't support multiple IDs).NB: to get the Fileset ID from an Image in webclient, see ome/omero-web#385