-
Notifications
You must be signed in to change notification settings - Fork 2
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
Record the tile shape in our data structures if it's not provided. #73
Conversation
If the tile_shape is not provided by the json file, we should infer it from the actual data. This is a suboptimal data path since it requires a decoding step just to get the shape of the tile, so we warn about it. Previously, this field was set by `_load()`, but that was removed in #72. Test plan: Generate a tileset document, and then strip the shape data from the tiles. Then read it back. The correct tile shapes should be provided, but getting them should trigger a warning.
ce0c250
to
2eed4f5
Compare
Codecov Report
@@ Coverage Diff @@
## master #73 +/- ##
==========================================
+ Coverage 82.49% 82.72% +0.23%
==========================================
Files 18 18
Lines 514 521 +7
==========================================
+ Hits 424 431 +7
Misses 90 90
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment.
def tile_shape(self): | ||
if self._tile_shape is None: | ||
warnings.warn( | ||
"Decoding tile just to obtain shape. It is recommended to include the tile shape " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a FieldOfView.json, right? Does it make more sense to call it by name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FieldOfView is a specific subset of TileSet in my mind.
Both notebooks had datasets where the tile shapes were not consistently provided. spacetx/slicedimage#72 broke the recording of the tile shape, and spacetx/slicedimage#73 fixes it. Test plan: `make -j run__notebooks/py/ISS_Pipeline_-_Breast_-_1_FOV.py run__notebooks/py/assay_comparison.py`
Both notebooks had datasets where the tile shapes were not consistently provided. spacetx/slicedimage#72 broke the recording of the tile shape, and spacetx/slicedimage#73 fixes it. Test plan: `make -j run__notebooks/py/ISS_Pipeline_-_Breast_-_1_FOV.py run__notebooks/py/assay_comparison.py`
If the tile_shape is not provided by the json file, we should infer it from the actual data. This is a suboptimal data path since it requires a decoding step just to get the shape of the tile, so we warn about it.
Previously, this field was set by
_load()
, but that was removed in #72.Test plan: Generate a tileset document, and then strip the shape data from the tiles. Then read it back. The correct tile shapes should be provided, but getting them should trigger a warning.