Add schema for tiled pixelmap overlays #767
Conversation
|
One thought is on naming here. Could we simplify to “image” and “pixelmap”? If the tiled nature is important to call out, maybe “tiledimage” and “tiledpixelmap” for consistency? |
| categoryColorMap = { | ||
| 'type': 'object', | ||
| 'properties': { | ||
| 'segments': { |
There was a problem hiding this comment.
I think this would be better as
'values': {
'type': 'array',
'items': {'type': 'integer'},
}
Using values for consistency with other annotations, and storing just indices into the category map rather than strings. Since we could have millions of pixels, using integers here will be more efficient.
The colormap might conceptually be better called categories where each entry has a color, a label string, and an optional description to give a more verbose description of the category.
For pixelmaps with boundaries, perhaps this value array should be half as long as the actual number of number of distinct indices and then the category would have a fillColor and a strokeColor (and for non-boundaries pixelmaps, this would just use the fillColor).
There was a problem hiding this comment.
I've implemented these changes in 47c74f4.
I've also removed having two representations of the values array. Let me know if there are any further refinements to be made.
Change `segments` to `values` to keep consistent with other annotation types. Also store integers in the `values` array instead of strings to save space. Those integers act as keys to indices in the `categories` array. Updated docs in line with changes.
| @@ -679,6 +733,7 @@ def _migrateACL(self, annotation): | |||
| return annotation | |||
|
|
|||
| def createAnnotation(self, item, creator, annotation, public=None): | |||
| print('\nCREATING ANNOTATION\n') | |||
There was a problem hiding this comment.
If you want to keep log messages, you should be able to use logger.debug (or another appropriate level). For development, I'll adjust the level to info and then set it to debug once I think it is stable.
There was a problem hiding this comment.
Removed in 24115d5
Also remove some extraneous print statements.
| }, | ||
| 'boundaries': { | ||
| 'type': 'boolean', | ||
| 'description': 'True if the boundaries of the pixelmap regions have ' |
There was a problem hiding this comment.
I don't think this description is right any more. Since we've added strokeColor into the category, this should probably read something on order of "True if the pixelmap doubles index values such that even values are the fill and odd values are the stroke of each superpixel. In this case, the values array will be half the length of the actual index values in the image".
There was a problem hiding this comment.
Updated description in 95bab98
| }, | ||
| }, | ||
| 'required': ['values', 'categories', 'boundaries'], | ||
| 'description': 'A tiled pixelmap to overlay onto a base resource.' |
There was a problem hiding this comment.
Please add 'additionalProperties': False to this and the pixelmapCategorySchema.
| @@ -437,6 +437,59 @@ class AnnotationSchema: | |||
| 'description': 'An image overlay on top of the base resource.', | |||
| }) | |||
|
|
|||
| pixelMapCategorySchema = { | |||
There was a problem hiding this comment.
For consistency, don't capitalize Map in pixelmap.
Also change some variable names for consistency and add additionalProperties to pixelmapCategorySchema and pixelmapSchema.
This PR includes the following changes:
tiledpixelmapelementstiledpixelmapelements using the same technique asimageoverlayelements