Skip to content

Move image loading logic to _load_image in VOC datasets (similar to COCO) #4975

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

Open
vadimkantorov opened this issue Nov 22, 2021 · 3 comments

Comments

@vadimkantorov
Copy link

vadimkantorov commented Nov 22, 2021

🚀 The feature

This would allow simple stubbing out image loading if needed for fast dataset transformed metadata access (when image reading is not needed) by dataset._load_image = lambda _: None and also make API more consistent across datasets.

It's also impossible to monkey-patch __getitem__ at instance level: https://stackoverflow.com/questions/57413453/is-it-possible-to-override-getitem-at-instance-level-in-python :(

Motivation, pitch

N/A

Alternatives

No response

Additional context

No response

@NicolasHug
Copy link
Member

I would recommend to inherit from the class, and override __getitem__ instead then, it should only be a few more lines of code.
If you submit a PR for this I think we would merge it, but keep in mind the _load_image function is private API, so while I know you know what you're doing, we don't offer any guarantee w.r.t. BC or consistency across datasets here.

@vadimkantorov
Copy link
Author

vadimkantorov commented Nov 23, 2021

Here's what I do now: dataset.__getitem__ = lambda index: dataset.transforms(None, dataset.parse_voc_xml(torchvision.datasets.voc.ET_parse(dataset.annotations[index]).getroot()))

Overriding getitem would entail to the same thing - having to copy-paste the even less standard / error-prone annotation parsing line.

Not wanting the dataset to load images for only metadata purposes is a valid and frequent usecase (often happens for some stats computation and avoiding duplicating ground truth preparation functions). At least simplifying hacking around would be helpful and minimize the copy-pasting. Accepting a method for image loading by a given path in __init__ is another way (probably for pickle purposes, there would need to be a torchvision method function None)

@pmeier
Copy link
Collaborator

pmeier commented Dec 10, 2021

As discussed in #4991, I'm ok with moving the functionality into a private method on the dataset as long as it is a 1-to-1 copy of the current behavior. Still, the BC part of #4975 (comment) applies.

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