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

Name option #147

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
+53 −20
Open

Name option #147

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e882a62
Add name_by option to allow name.ome.zarr
will-moore Jul 11, 2023
d80d276
Remove .pattern from image name
will-moore Jul 11, 2023
1d72626
Handle first plate Wells having no fields
will-moore Aug 7, 2023
6290b4b
Don't try to remove .pattern from image names
will-moore Aug 8, 2023
c73d400
Replace [] with () in zarr names
will-moore Aug 15, 2023
57da8bb
Add get_zarr_name(). Always use .ome.zarr
will-moore Aug 23, 2023
36e66c4
Update README with --name_by option
will-moore Aug 23, 2023
3ca2c58
Update README and masks.py to use ID.ome.zarr
will-moore Aug 23, 2023
ecf5a3e
Merge remote-tracking branch 'origin/master' into name_option
will-moore Oct 24, 2023
470ad2b
Use .ome.zarr in args help
will-moore Dec 14, 2023
a14f62a
Improve source-image example in README
will-moore Dec 14, 2023
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
9 changes: 9 additions & 0 deletions src/omero_zarr/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,15 @@ def _configure(self, parser: Parser) -> None:
default=None,
help="Maximum number of workers (only for use with bioformats2raw)",
)
export.add_argument(
"--name_by",
default="id",
choices=["id", "name"],
help=(
"How to name the Image or Plate zarr. Default 'id' is [ID].zarr. "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That raises the question about the extension.
In both cases, the library is used to generate ome.zarr so in one case using .zarr and in the other case .ome.zarr is confusing

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general I think we've stated elsewhere:

  • strongly SHOULD (bordering on MUST): .zarr
  • SHOULD: .ome.zarr

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joshmoore You're suggesting we update the default behaviour to be [ID].ome.zarr instead of [ID].zarr?
Always using .ome.zarr (for name.ome.zarr or ID.ome.zarr) is more consistent, so I'm happy to do that, but that is more of a breaking change

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breaking in the sense that you think current processes won't detect that something has already been exported and will re-export?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, just if you've got a workflow like:

$ omero zarr export Image:1
$ mv 1.zarr renamed_image.zarr

that workflow will need to change because 1.zarr will now be created as 1.ome.zarr

But that's not a major concern, so happy to make that change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I'm personally a bit up in the air. A benefit of .zarr filesets are that other stuff can be stored in them which is why I don't think we should ever require .ome.zarr. On the flip side, what we do now will likely be the model that other submitters to BIA tend to follow, so it does make sense to do what we think is most strategic (and update any existing workflows as necessary). All that being said, 💯 for doing whatever consistently.

"'name' is [NAME].ome.zarr"
),
)
export.add_argument(
"object",
type=ProxyStringType("Image"),
Expand Down
18 changes: 15 additions & 3 deletions src/omero_zarr/raw_pixels.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,14 @@ def image_to_zarr(image: omero.gateway.ImageWrapper, args: argparse.Namespace) -
target_dir = args.output
tile_width = args.tile_width
tile_height = args.tile_height

name = os.path.join(target_dir, "%s.zarr" % image.id)
name_by = args.name_by

if name_by == "name":
# remove .pattern from name, add .ome.zarr
img_name = image.name.replace(".pattern", "")
name = os.path.join(target_dir, "%s.ome.zarr" % img_name)
else:
name = os.path.join(target_dir, "%s.zarr" % image.id)
print(f"Exporting to {name} ({VERSION})")
store = open_store(name)
root = open_group(store)
Expand Down Expand Up @@ -245,7 +251,13 @@ def plate_to_zarr(plate: omero.gateway._PlateWrapper, args: argparse.Namespace)
total = n_rows * n_cols * (n_fields[1] - n_fields[0] + 1)

target_dir = args.output
name = os.path.join(target_dir, "%s.zarr" % plate.id)
name_by = args.name_by

if name_by == "name":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two quick notes:

  • this code is duplicated and could be made a method for possible sub-classing
  • I can live with this split in naming schemes, but it could be a surprise

name = os.path.join(target_dir, "%s.ome.zarr" % plate.name)
else:
name = os.path.join(target_dir, "%s.zarr" % plate.id)

store = open_store(name)
print(f"Exporting to {name} ({VERSION})")
root = open_group(store)
Expand Down