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.

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

Fix dataset paths when --scale-format-string is used #181

Merged

Conversation

melissalinkert
Copy link
Member

Fixes #179.

This removes trailing slashes (as would be present with --scale-format-string '%2$d/') so that the relative dataset path is not empty.

The associated test should fail without the change to Converter, and pass with it. Independent of the test, something like the command in #179 can be used:

$ bioformats2raw -p input.fake output.zarr --scale-format-string '%2$d/'
$ bioformats2raw -p input.fake output-minimal.zarr --scale-format-string '%2$d/' --no-root-group --no-ome-meta-export

Without this PR, in both cases the *.zarr/.zattrs should have empty path attributes on each of the datasets. With this PR, both commands should result in path being populated with something that exists relative to the .zattrs location.

Fixes glencoesoftware#179.

This removes trailing slashes (as would be present with `--scale-format-string '%2$d/'`)
so that the relative dataset path is not empty.
Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

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

The change looks good and adds support for the presence of a trailing slash (which is the default example in the README).

Confirmed the additional asserts introduced in the unit tests fail without the Converter fix when trailing slashes are appended and pass with the fix.

Also tested locally using the two scenarios proposed in the description

(java11) sbesson@Sebastiens-MacBook-Pro-2 bioformats2raw % ./bioformats2raw-0.6.0/bin/bioformats2raw test.fake  --scale-format-string '%2$d/' out_060.zarr
(java11) sbesson@Sebastiens-MacBook-Pro-2 bioformats2raw % ./bioformats2raw-0.6.0/bin/bioformats2raw test.fake --scale-format-string '%2$d/' out_060_minimal.zarr  --no-root-group --no-ome-meta-export
(java11) sbesson@Sebastiens-MacBook-Pro-2 bioformats2raw % ./bioformats2raw-0.7.0-SNAPSHOT/bin/bioformats2raw test.fake  --scale-format-string '%2$d/' out_070_SNAPSHOT.zarr
(java11) sbesson@Sebastiens-MacBook-Pro-2 bioformats2raw % ./bioformats2raw-0.7.0-SNAPSHOT/bin/bioformats2raw test.fake  --scale-format-string '%2$d/' out_070_SNAPSHOT_minimal.zarr  --no-root-group --no-ome-meta-export

A recursive diff indicates the only difference is the value of the path fields in the multiscales metadata which are updated from empty strings to valid paths

(java11) sbesson@Sebastiens-MacBook-Pro-2 bioformats2raw % diff -ur out_060.zarr out_070_SNAPSHOT.zarr 
diff -ur out_060.zarr/.zattrs out_070_SNAPSHOT.zarr/.zattrs
--- out_060.zarr/.zattrs	2022-12-12 09:47:52.000000000 +0000
+++ out_070_SNAPSHOT.zarr/.zattrs	2022-12-12 09:48:22.000000000 +0000
@@ -22,13 +22,13 @@
     } ],
     "name" : "test",
     "datasets" : [ {
-      "path" : "",
+      "path" : "0",
       "coordinateTransformations" : [ {
         "scale" : [ 1.0, 1.0, 1.0, 1.0, 1.0 ],
         "type" : "scale"
       } ]
     }, {
-      "path" : "",
+      "path" : "1",
       "coordinateTransformations" : [ {
         "scale" : [ 1.0, 1.0, 1.0, 2.0, 2.0 ],
         "type" : "scale"
(java11) sbesson@Sebastiens-MacBook-Pro-2 bioformats2raw % diff -ur out_060_minimal.zarr out_070_SNAPSHOT_minimal.zarr 
diff -ur out_060_minimal.zarr/.zattrs out_070_SNAPSHOT_minimal.zarr/.zattrs
--- out_060_minimal.zarr/.zattrs	2022-12-12 09:48:01.000000000 +0000
+++ out_070_SNAPSHOT_minimal.zarr/.zattrs	2022-12-12 09:48:43.000000000 +0000
@@ -22,13 +22,13 @@
     } ],
     "name" : "test",
     "datasets" : [ {
-      "path" : "",
+      "path" : "0",
       "coordinateTransformations" : [ {
         "scale" : [ 1.0, 1.0, 1.0, 1.0, 1.0 ],
         "type" : "scale"
       } ]
     }, {
-      "path" : "",
+      "path" : "1",
       "coordinateTransformations" : [ {
         "scale" : [ 1.0, 1.0, 1.0, 2.0, 2.0 ],
         "type" : "scale"

@chris-allan chris-allan merged commit 08a24f3 into glencoesoftware:master Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't remove series from directory structure with --scale-format-string
3 participants