-
Notifications
You must be signed in to change notification settings - Fork 13
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
made Importer `zarr´ compatible #99
made Importer `zarr´ compatible #99
Conversation
Co-Authored-By: Will Moore <[email protected]>
ezomero/_importer.py
Outdated
@@ -468,6 +470,7 @@ def ezimport_ln_s(self) -> bool: | |||
'-k', self.conn.getSession().getUuid().val, | |||
'-s', self.conn.host, | |||
'-p', str(self.conn.port), | |||
'--depth', '10', |
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'm going to suggest that, instead of hardcoding that, ezimport
can take extra arguments that get passed through to the import
call at CLI level. This way, if we ever need --depth 20
, or if folks need to pass any other optional import arguments, that doesn't require any code change.
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.
@erickmartins thanks for the feedback. Do you mean adding it as a keyword argument to the Importer
class?
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 mean adding *args, **kwargs
to the ezimport
function signature, and passing these through to the omero import
call there (changing imp_ctl.ezimport()
to imp_ctl.ezimport(*args, **kwargs)
). In fact, that would also mean getting rid of the ln_s
optional parameter and the ezimport_ln_s
method.
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.
db4f409 is more or less what I had in mind here - it means users can pass extra arguments directly into ezimport
and they get sent to omero import
. in that case, making it work for zarrs would just mean calling ezimport(conn, "/path/to/file.zarr", depth=10)
, doing in-place imports would just be calling ezimport(conn, "/path/to/file.tiff", transfer="ln_s")
and so on.
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 for working on this! I'm on parental leave until early April so thanks big-time for the fix :)
Description
Fixes #97
This PR implements the fixes suggested by @will-moore in this image.sc thread to allow for the import of
.zarr
files into OMERO throughezomero
.For reviewers
later.
__init__.py
.