AllenCellModeling / aicspylibczi

Python module utilizing libCZI for reading Zeiss CZI files.
https://allencellmodeling.github.io/aicspylibczi
GNU General Public License v3.0
36 stars 7 forks source link

Issues with understanding the naming of "scene_bounding_box" and "scene_height_by_width" functions #66

Open sebi06 opened 3 years ago

sebi06 commented 3 years ago

Hi,

i used the following function over the last days and got a bit confused by the naming for the following functions

So why is the function called scene_bounding_box, because it does not return the BBox of the specific scene. Or did I misunderstood something here.

I tested this on a CZI file with 4 different scenes (different size and shape), where a single tile is always 640x640. The output is below. Or do I misinterpret the naming of those function. To me it looks like the term tile (many tile make up a scence)) is mixed up with the term scene (a scene has many tiles) a bit, isn't it?

Cheers, Sebi

Mosaic Size:  (31272, 36565, 66462, 9878)
-----------------------------------------------------
scene_bounding_box     :  (34152, 37246, 640, 640)
scene_height_by_width :  (640, 640)
Scene BBox (own code) :  31272 37246 3520 5248
BBox Scene:  0 (1, 5248, 3520)
-----------------------------------------------------
scene_bounding_box     :  (52929, 40619, 640, 640)
scene_height_by_width :  (640, 640)
Scene BBox (own code) :  51777 40619 6400 5824
BBox Scene:  1 (1, 5824, 6400)
-----------------------------------------------------
scene_bounding_box     :  (73421, 42068, 640, 640)
scene_height_by_width :  (640, 640)
Scene BBox (own code) :  73421 42068 1792 2944
BBox Scene:  2 (1, 2944, 1792)
-----------------------------------------------------
scene_bounding_box     :  (94790, 36565, 640, 640)
scene_height_by_width :  (640, 640)
Scene BBox (own code) :  94790 36565 2944 1792
BBox Scene:  3 (1, 1792, 2944)
-----------------------------------------------------

Figure_1

heeler commented 3 years ago

Hi @sebi06, you are correct. When I first implemented this we really weren't supporting mosaic files and were working with subblocks that spanned a scene and thus the current naming is a little odd. I've started working on a 3.0 version and will address changes in there. Once I have the API in a PR I will add you as a reviewer if you don't mind. I appreciate your pointing this out.

evamaxfield commented 3 years ago

@heeler

I would rename:

evamaxfield commented 3 years ago

All functions start with verb all props start with noun or static value: