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

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

Open
sebi06 opened this issue Dec 15, 2020 · 3 comments
Assignees
Milestone

Comments

@sebi06
Copy link

sebi06 commented Dec 15, 2020

Hi,

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

  • mosaic_scene_bounding_boxes(index=SceneID)

  • scene_bounding_box(index=SceneID)

  • scene_height_by_width(index=SceneID)

  • 1st yields in a List with tuples for every tile of a specific scene = SceneID

  • 2nd yields in a tuple for the first tile of a specific scene = SceneID with (xmin, ymin, width-of-tiles, height-of-tiles)

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.

  • 3rd returns the width and height of the tiles of the scene = SceneID. Is this correct and if yes, should it be calle scene_tile_height_by_width etc?

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
Copy link
Member

heeler commented Jan 29, 2021

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.

@heeler heeler self-assigned this Jan 29, 2021
@heeler heeler added this to the 3.0 milestone Jan 29, 2021
@evamaxfield
Copy link
Collaborator

@heeler

I would rename:

  • mosaic_scene_bounding_boxes(index=SceneID) -> get_all_mosaic_tile_bounding_boxes
  • scene_bounding_box -> get_single_tile_bounding_info
  • scene_height_by_width -> get_single_tile_height_by_width

@evamaxfield
Copy link
Collaborator

evamaxfield commented Mar 1, 2021

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

  • function to get image: get_image_data or is_supported_image
  • property as shorthand for access to data: data or current_scene

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants