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

Rename labels #64

Merged
merged 3 commits into from
Aug 19, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
31 changes: 16 additions & 15 deletions spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,19 @@ for public re-use.
│ └── t.c.z.y.x # a "chunk coordinate", where the maximum coordinate
│ # will be `dimension_size / chunk_size`.
└── masks
└── labels
├── .zgroup # The masks group is a container which holds a list
├── .zattrs # of masks to make the objects easily discoverable,
│ # All masks will be listed in `.zattrs` e.g. `{ "masks": [ "original/0" ] }`
│ # Each dimension of the mask `(t, c, z, y, x)` should be either the same as the
│ # corresponding dimension of the image, or `1` if that dimension of the mask
├── .zgroup # The labels group is a container which holds a list
├── .zattrs # of labels to make the objects easily discoverable,
│ # All labels will be listed in `.zattrs` e.g. `{ "labels": [ "original/0" ] }`
│ # Each dimension of the label `(t, c, z, y, x)` should be either the same as the
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
│ # Each dimension of the label `(t, c, z, y, x)` should be either the same as the
│ # Each dimension of the label `(t, c, z, y, x)` must be either the same as the

Is there a situation where the dimension would be something else?

Copy link
Member Author

Choose a reason for hiding this comment

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

#66

│ # corresponding dimension of the image, or `1` if that dimension of the label
│ # is irrelevant.
└── original # Intermediate folders are permitted but not necessary
│ # and currently contain no extra metadata.
└── 0
├── .zarray # Each mask itself is a 5D array matching the highest resolution
├── .zarray # Each label itself is a 5D array matching the highest resolution
└── .zattrs # of the related image and has an extra key, "color", with display information.


Expand Down Expand Up @@ -113,20 +113,20 @@ can be found under the "omero" key in the group-level metadata:
See https://docs.openmicroscopy.org/omero/5.6.1/developers/Web/WebGateway.html#imgdata
for more information.

### "masks"
### "labels"

The special group "masks" found under an image Zarr contains the key `masks` containing
the paths to mask objects which can be found underneath the group:
The special group "labels" found under an image Zarr contains the key `labels` containing
the paths to label objects which can be found underneath the group:

```
{
"masks": [
"labels": [
"orphaned/0"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"orphaned/0"
"original/0"

Match the example above? Or was this intentionally different to indicate something else?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we probably need a few examples. Not also that the omero-{cli,ms}-zarr currently use the unimaginative "0" as the default name. To not need to fight with travis again, I'd propose we have a round of "label-naming" after things have settled a bit.

Copy link
Member Author

Choose a reason for hiding this comment

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

#67

]
}
```

Unlisted groups MAY be masks.
Unlisted groups MAY be labels.

### "color"

Expand All @@ -143,7 +143,7 @@ the value is an RGBA color (4 byte, `0-255` per channel) for representing the ob
```
### "image"

The `image` key is an optional dictionary which contains information on the image the mask is associated with.
The `image` key is an optional dictionary which contains information on the image the label is associated with.
If included it must include a key `array` whose value that is either:
- A relative path to a Zarr image array, for example:
```
Expand All @@ -153,7 +153,7 @@ If included it must include a key `array` whose value that is either:
}
}
```
- A URL to a Zarr image array (use this if the mask is stored seperately from the image Zarr), for example:
- A URL to a Zarr image array (use this if the label is stored seperately from the image Zarr), for example:
```
{
"image": {
Expand All @@ -166,7 +166,8 @@ If included it must include a key `array` whose value that is either:

| Revision | Date | Description |
| ---------- | ------------ | ------------------------------------------ |
| 0.1.3 | 2020-07-07 | Add mask metadata |
| 0.1.3 | 2020-08-18 | Rename masks as labels |
| 0.1.3-dev1 | 2020-07-07 | Add mask metadata |
| 0.1.2 | 2020-05-07 | Add description of "omero" metadata |
| 0.1.1 | 2020-05-06 | Add info on the ordering of resolutions |
| 0.1.0 | 2020-04-20 | First version for internal demo |
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,18 @@ public class RequestHandlerForImage implements HttpHandler {

private static final Logger LOGGER = LoggerFactory.getLogger(RequestHandlerForImage.class);

public static final String DEFAULT_LABEL_NAME = "0"; // See omero-cli-zarr

/**
* Key (i.e. path) to the group containing the label arrays.
*/
public static final String LABEL_GROUP = "labels"; // See omero-cli-zarr

/**
* Key (i.e. JSON key) to the metadata for the label arrays.
*/
public static final String LABEL_KEY = LABEL_GROUP;

/**
* Contains the dimensionality of an image.
* @author [email protected]
Expand Down Expand Up @@ -360,24 +372,24 @@ public RequestHandlerForImage(Configuration configuration, PixelsService pixelsS
this.patternForImageDir = Pattern.compile(path);
this.patternForImageGroupDir = Pattern.compile(path + "(\\d+)/");
this.patternForImageChunkDir = Pattern.compile(path + "(\\d+)/(\\d+([/.]\\d+)*)/");
this.patternForImageMasksDir = Pattern.compile(path + "masks/");
this.patternForMaskDir = Pattern.compile(path + "masks/(\\d+)/");
this.patternForMaskChunkDir = Pattern.compile(path + "masks/(\\d+)/(\\d+([/.]\\d+)*)/");
this.patternForMaskLabeledDir = Pattern.compile(path + "masks/labell?ed/");
this.patternForMaskLabeledChunkDir = Pattern.compile(path + "masks/labell?ed/(\\d+([/.]\\d+)*)/");
this.patternForImageMasksDir = Pattern.compile(path + LABEL_GROUP + "/");
this.patternForMaskDir = Pattern.compile(path + LABEL_GROUP + "/(\\d+)/");
this.patternForMaskChunkDir = Pattern.compile(path + LABEL_GROUP + "/(\\d+)/(\\d+([/.]\\d+)*)/");
this.patternForMaskLabeledDir = Pattern.compile(path + LABEL_GROUP + "/" + DEFAULT_LABEL_NAME + "/");
Copy link
Member

Choose a reason for hiding this comment

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

Works okay in this case but among these may want to consider \\Q\\E as in Configuration.getRegexForNetPath if there is any risk of a regex metacharacter someday appearing, e.g., if later allowing these to be configurable. Not a blocker though.

Copy link
Member Author

Choose a reason for hiding this comment

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

#69

this.patternForMaskLabeledChunkDir = Pattern.compile(path + LABEL_GROUP + "/" + DEFAULT_LABEL_NAME + "/(\\d+([/.]\\d+)*)/");

this.patternForImageGroup = Pattern.compile(path + "\\.zgroup");
this.patternForImageAttrs = Pattern.compile(path + "\\.zattrs");
this.patternForImageMasksGroup = Pattern.compile(path + "masks/\\.zgroup");
this.patternForImageMasksAttrs = Pattern.compile(path + "masks/\\.zattrs");
this.patternForMaskAttrs = Pattern.compile(path + "masks/(\\d+)/\\.zattrs");
this.patternForMaskLabeledAttrs = Pattern.compile(path + "masks/labell?ed/\\.zattrs");
this.patternForImageMasksGroup = Pattern.compile(path + LABEL_GROUP + "/\\.zgroup");
this.patternForImageMasksAttrs = Pattern.compile(path + LABEL_GROUP + "/\\.zattrs");
this.patternForMaskAttrs = Pattern.compile(path + LABEL_GROUP + "/(\\d+)/\\.zattrs");
this.patternForMaskLabeledAttrs = Pattern.compile(path + LABEL_GROUP + "/" + DEFAULT_LABEL_NAME + "/\\.zattrs");
this.patternForImageArray = Pattern.compile(path + "(\\d+)/\\.zarray");
this.patternForMaskArray = Pattern.compile(path + "masks/(\\d+)/\\.zarray");
this.patternForMaskLabeledArray = Pattern.compile(path + "masks/labell?ed/\\.zarray");
this.patternForMaskArray = Pattern.compile(path + LABEL_GROUP + "/(\\d+)/\\.zarray");
this.patternForMaskLabeledArray = Pattern.compile(path + LABEL_GROUP + "/" + DEFAULT_LABEL_NAME + "/\\.zarray");
this.patternForImageChunk = Pattern.compile(path + "(\\d+)/(\\d+([/.]\\d+)*)");
this.patternForMaskChunk = Pattern.compile(path + "masks/(\\d+)/(\\d+([/.]\\d+)*)");
this.patternForMaskLabeledChunk = Pattern.compile(path + "masks/labell?ed/(\\d+([/.]\\d+)*)");
this.patternForMaskChunk = Pattern.compile(path + LABEL_GROUP + "/(\\d+)/(\\d+([/.]\\d+)*)");
this.patternForMaskLabeledChunk = Pattern.compile(path + LABEL_GROUP + "/" + DEFAULT_LABEL_NAME + "/(\\d+([/.]\\d+)*)");
}

/**
Expand Down Expand Up @@ -660,7 +672,7 @@ private void returnImageDirectory(HttpServerResponse response, List<String> para
contents.add(".zgroup");
for (final long roiId : omeroDao.getRoiIdsOfImage(imageId)) {
if (omeroDao.getMaskCountOfRoi(roiId) > 0) {
contents.add("masks/");
contents.add(LABEL_GROUP + "/");
break;
}
}
Expand Down Expand Up @@ -763,7 +775,7 @@ private void returnImageMasksDirectory(HttpServerResponse response, List<String>
final ImmutableList.Builder<String> contents = ImmutableList.builder();
contents.add(".zattrs");
if (labeledMasks != null) {
contents.add("labeled/");
contents.add(DEFAULT_LABEL_NAME);
Copy link
Member

Choose a reason for hiding this comment

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

Still needs a terminal "/"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Part of me says we should support both but the removal was unintentional. Thanks.

joshmoore marked this conversation as resolved.
Show resolved Hide resolved
}
if (isSplitMasksEnabled) {
for (long roiId : roiIdsWithMask) {
Expand Down Expand Up @@ -1140,7 +1152,7 @@ private void returnImageMasksAttrs(HttpServerResponse response, List<String> par
}
final List<String> masks = new ArrayList<>(roiIdsWithMask.size() + 1);
if (labeledMasks != null) {
masks.add("labeled");
masks.add(DEFAULT_LABEL_NAME);
}
if (isSplitMasksEnabled) {
for (final long roiId : roiIdsWithMask) {
Expand All @@ -1149,7 +1161,7 @@ private void returnImageMasksAttrs(HttpServerResponse response, List<String> par
}
/* package data for client */
final Map<String, Object> result = new HashMap<>();
result.put("masks", masks);
result.put(LABEL_KEY, masks);
respondWithJson(response, new JsonObject(result));
}

Expand Down
10 changes: 6 additions & 4 deletions src/test/java/org/openmicroscopy/ms/zarr/ZarrBinaryMaskTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
package org.openmicroscopy.ms.zarr;

import org.openmicroscopy.ms.zarr.mask.ImageMask;
import static org.openmicroscopy.ms.zarr.RequestHandlerForImage.DEFAULT_LABEL_NAME;
import static org.openmicroscopy.ms.zarr.RequestHandlerForImage.LABEL_GROUP;

import ome.model.core.Image;
import ome.model.core.Pixels;
Expand Down Expand Up @@ -134,7 +136,7 @@ protected OmeroDao daoSetup() {
@Test
public void testMaskChunks() throws DataFormatException, IOException {
final Set<Byte> seenIsMasked = new HashSet<>();
final JsonObject response = getResponseAsJson(image.getId(), "masks", roi1.getId(), ".zarray");
final JsonObject response = getResponseAsJson(image.getId(), LABEL_GROUP, roi1.getId(), ".zarray");
final JsonArray shape = response.getJsonArray("shape");
final int maskSizeX = shape.getInteger(4);
final int maskSizeY = shape.getInteger(3);
Expand All @@ -149,7 +151,7 @@ public void testMaskChunks() throws DataFormatException, IOException {
for (int x = 0; x < maskSizeX; x += chunkSizeX) {
mockSetup();
final byte[] chunkZipped =
getResponseAsBytes(image.getId(), "masks", roi1.getId(), 0, 0, 0, chunkIndexY, chunkIndexX);
getResponseAsBytes(image.getId(), LABEL_GROUP, roi1.getId(), 0, 0, 0, chunkIndexY, chunkIndexX);
final byte[] chunk = uncompress(chunkZipped);
for (int cx = 0; cx < chunkSizeX; cx++) {
for (int cy = 0; cy < chunkSizeY; cy++) {
Expand Down Expand Up @@ -178,7 +180,7 @@ public void testMaskChunks() throws DataFormatException, IOException {
@Test
public void testLabeledMaskChunks() throws DataFormatException, IOException {
final Set<Long> seenLabels = new HashSet<>();
final JsonObject response = getResponseAsJson(image.getId(), "masks", "labeled", ".zarray");
final JsonObject response = getResponseAsJson(image.getId(), LABEL_GROUP, DEFAULT_LABEL_NAME, ".zarray");
final JsonArray shape = response.getJsonArray("shape");
final int maskSizeX = shape.getInteger(4);
final int maskSizeY = shape.getInteger(3);
Expand All @@ -192,7 +194,7 @@ public void testLabeledMaskChunks() throws DataFormatException, IOException {
int chunkIndexX = 0;
for (int x = 0; x < maskSizeX; x += chunkSizeX) {
mockSetup();
final byte[] chunkZipped = getResponseAsBytes(image.getId(), "masks", "labeled", 0, 0, 0, chunkIndexY, chunkIndexX);
final byte[] chunkZipped = getResponseAsBytes(image.getId(), LABEL_GROUP, DEFAULT_LABEL_NAME, 0, 0, 0, chunkIndexY, chunkIndexX);
final ByteBuffer chunk = ByteBuffer.wrap(uncompress(chunkZipped));
for (int cx = 0; cx < chunkSizeX; cx++) {
for (int cy = 0; cy < chunkSizeY; cy++) {
Expand Down