ENH: Improve PlaneSegmentation.add_roi doc

[rly]

Motivation

The docstring for PlaneSegmentation.add_roi(...) should describe why/when to use image_mask vs pixel_mask or voxel_mask.

Based on feedback from the Allen Institute.

Checklist

• Have you checked our Contributing document?
• Have you ensured the PR description clearly describes problem and the solution?
• Is your contribution compliant with our coding style? This can be checked running flake8 from the source directory.
• Have you checked to ensure that there aren't other open Pull Requests for the same change?
• Have you included the relevant issue number using #XXX notation where XXX is the issue number?

[bendichter]

IMO I'd prefer image_mask over pixel_mask in all cases. If the image_mask is numerically sparse, gzip should be able to take care of that. image_mask + gzip will probably take up even less space than unzipped pixel_mask, especially if the image_mask data in binary. If you want to go next level, you could also set the fill_value to 0 and only fill in the chunks that contain some of the ROI. I can't think of a good compelling case for using pixel_mask over image_mask

[oruebel]

@bendichter with regard to storage in the file that is true, but keep in mind that in general, the same optimizations (chunking, compression) are not available in memory. I.e., once a user has to load the data pixel_mask will in general be more efficient for sparse ROI's in memory than image_mask. For small images the difference is probably not important but for large-images full image masks per ROI can become very expensive.

[bendichter]

@oruebel ok that's a good point

[rly]

Also, applying compression to an NWB file is an advanced step that I think many users are not yet doing. If we are going to encourage usage that is optimized for the compressed case, then we should make sure to encourage compression in the tutorials and documentation.

I updated the docstring again to be more clear and address the points you raised. Feel free to edit or make further suggestions.

[bendichter]

Are we suggesting that users compress the entire NWB file? Is there a way of just compressing the image masks?

[rly]

Good point. I will add 'data' to the allowed types for the masks and test this out.

[codecov]

Codecov Report

Merging #1308 (710972b) into dev (734a6c4) will increase coverage by 8.32%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##              dev    #1308      +/-   ##
==========================================
+ Coverage   83.66%   91.98%   +8.32%     
==========================================
  Files          27       27              
  Lines        2608     2608              
  Branches      681      681              
==========================================
+ Hits         2182     2399     +217     
+ Misses        339      134     -205     
+ Partials       87       75      -12     

Flag	Coverage Δ
integration	71.24% <0.00%> (?)
unit	83.66% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
src/pynwb/ophys.py	96.05% <0.00%> (+1.97%)	⬆️

... and 13 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more