Skip to content
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.

Sign up for GitHub

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

Jump to bottom

made Importer `zarr´ compatible #99

Merged
merged 2 commits into from
Apr 1, 2024
Merged

made Importer `zarr´ compatible #99

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
03cd2ae
made Importer `zarr´ compatible
jo-mueller Jan 30, 2024
db4f409
passing *args, **kwargs to `omero import`
erickmartins Mar 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ezomero/_importer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,8 @@ def get_my_image_ids(self) -> Union[List[int], None]:
params = Parameters()
path_query = self.make_substitutions()
path_query = path_query.strip('/')
if path_query.endswith(".zarr"):
path_query = f"{path_query}/.zattrs"
params.map = {"cpath": rstring(path_query)}
results = q.projection(
"SELECT i.id FROM Image i"
Expand Down Expand Up @@ -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',
Copy link
Collaborator

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.

Copy link
Contributor Author

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?

Copy link
Collaborator

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.

Copy link
Collaborator

@erickmartins erickmartins Mar 20, 2024 •
edited
Loading

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.

Copy link
Contributor Author

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 :)

'--transfer', 'ln_s',
str(self.file_path)])
if cli.rv == 0:
Expand Down
Loading