-
Notifications
You must be signed in to change notification settings - Fork 10
Import command #180
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
Import command #180
Conversation
|
This looks like similar issues to those discussed at #172 (comment) (also still failing checks there)... "The jobs above are failing on the mypy check being run by omero-test-infra, so that's where we need to fix the python version. How do I find out the version of python used by omero-test-infra, and can I control it? The issue of python-version for omero-test-infra was also asked about at ome/omero-web-zarr#19 (comment) cc @joshmoore |
|
Checks above are failing, but test is running and passing locally: |
|
Build errors seem to be due to Zarr v3 references: |
|
Conflicting PR. Removed from build OMERO-plugins-push#516. See the console output for more details.
|
dominikl
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.
Looks good to me! Thanks Will. I guess before mergeing we have to discuss/decide on the name import/register/??? .
|
Actually, just noticed that this doesn't seem to work with local files on the file system: |
|
@dominikl I think that looks like you have zarr v3 in your local env. You need zarr v2 |
|
Indeed. There's something odd going on. I simply checkout this branch and |
|
Sorry, installing zarr 3 does not uninstall omero-cli-zarr, it just break it: |
|
DON'T INSTALL ZARR V3. |
|
|
Sorry, my bad. It is a case of zarr 3 vv 2 mixup, and you do need zarr v2, but that line is wrong. Try |
|
Thanks Will, that fixes the problem! |
|
@dominikl I added that fix, and also ported the labels code from https://github.com/BioNGFF/omero-import-utils/pull/7/files. Tested with: |
src/omero_zarr/cli.py
Outdated
| ), | ||
| ) | ||
| register.add_argument( | ||
| import_cmd.add_argument( |
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.
I understood that we were not going to include labels for now as part of this workflow
|
@jburel I understood that we were going to prioritise the migration from BioNGFF/omero-import-utils to here ahead of merging the labels PR there (which we've done). |
BioNGFF/omero-import-utils#7 The PR was closed. We decided not to merge it due to the time it was taking, we have to read the binary data when supporting labels. I understand that it can be useful but I think we need more tests before pushing forward with that option |
|
The PR was only closed after copying it over to this PR. I just uploaded a new sample that is Imported into merge-ci...
I could revert that and move it to another branch, on top of this one, but it might be nicer to just do a bit more testing on this one? @dominikl |
|
I don't think it was solely closed because we started working on this repository. My preference:
|
dominikl
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.
Thanks, looks good to me. Tested with some examples from BioNGFF/omero-import-utils#13. Also no --labels option any longer, and trying to import plate says that it's not supported yet. 👍
Hopefully you can easily revert your last two commits and open separate PRs for them (I think there's a git command for it?).
|
suggestion: 0.7.0rc1 |
546b09a to
d00ba5e
Compare
|
@dominikl I just pushed a couple of commits to update the README, to add |
|
👍 Thanks, looks good to me! |

What do we call this command?
Please indicate your preferred name for the command. We will obviously need to add documentation
importrecordrecordimportimport]"This takes the
register.pyscript from BioNGFF/omero-import-utils#24 and adds it to omero-cli-zarr.This still gives us nice features:
omero zarr import ...from anywhere (don't have to cd toomero-import-utils/metadata)To test:
register_zarr(url)method.$ cd omero-cli-zarrand then$ pip install -e .