Skip to content

BounxingBoxes or BoundingBox #7313

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

Closed
NicolasHug opened this issue Feb 23, 2023 · 7 comments
Closed

BounxingBoxes or BoundingBox #7313

NicolasHug opened this issue Feb 23, 2023 · 7 comments

Comments

@NicolasHug
Copy link
Member

Opening so we don't forget: we should align ClampBoundingBox and Sanitize BoundingBoxes to both use either plural or singular

@mpearce25
Copy link
Contributor

Created a PR that

Standardize around singular "Box" as SanitizeBoundingBoxes was the only plural usage while the singular is used for all other classes.

lmk if it looks good

@pmeier
Copy link
Collaborator

pmeier commented Feb 24, 2023

I feel like the names are good as is. ClampBoundingBox can work with shapes (*, 4) (including singular (4,)) while SanitizeBoundingBoxes requires plural (N, 4).

@NicolasHug
Copy link
Member Author

There's nothing that prevents SanitizeBoundingBoxes to work on a singular (4,) right? We just haven't implemended yet?

@pmeier
Copy link
Collaborator

pmeier commented Feb 24, 2023

There's nothing that prevents SanitizeBoundingBoxes to work on a singular (4,) right?

How would the shape look like if you sanitize a (4,) bounding box? If you remove all bounding boxes from (N, 4), it becomes (0, 4).

@NicolasHug
Copy link
Member Author

Empty tensor?

[ins] In [2]: torch.tensor([]).shape
Out[2]: torch.Size([0])

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Feb 24, 2023

I would prefer to see everywhere BoundingBox so that there is no confusion in names between the subclass and the transforms

@NicolasHug
Copy link
Member Author

We synced offline and keeping singular seems to be OK with everyone. This was addressed in #7313, thanks for your contribution @mpearce25

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

4 participants