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

Places365 dataset #2610

Merged
merged 13 commits into from
Aug 25, 2020
Merged

Places365 dataset #2610

merged 13 commits into from
Aug 25, 2020

Conversation

pmeier
Copy link
Collaborator

@pmeier pmeier commented Aug 24, 2020

This adds the Places365 dataset. As the name implies it comprises 365 categories of "places". It is structured in 4 different splits:

  • training-standard (~ 1.8e6 images)
  • training-challenge (~ 8e6 images)
  • val (~ 18e3 images)
  • test (~ 330e3 images)

With the flag small the user can switch between high resolution and small (256 x 256) images.

@pmeier pmeier requested a review from fmassa August 24, 2020 14:26
Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice work, thanks a lot @pmeier !

I think this looks good to be merged, I have only a few questions / comments.

This PR is missing an entry in the documentation in https://github.com/pytorch/vision/blob/master/docs/source/datasets.rst , can you add one there (we have just sorted it alphabetically)

For a future PR, I think it might make sense to split the tests for Places365 in its own Tester class (still in the same file is ok), as we have quite a few test functions there.

torchvision/datasets/places365.py Outdated Show resolved Hide resolved
test/test_datasets.py Show resolved Hide resolved
test/test_datasets.py Show resolved Hide resolved
@pmeier
Copy link
Collaborator Author

pmeier commented Aug 25, 2020

For a future PR, I think it might make sense to split the tests for Places365 in its own Tester class (still in the same file is ok), as we have quite a few test functions there.

So basically

class Places365Tester(TestCase):
    pass

?

@fmassa
Copy link
Member

fmassa commented Aug 25, 2020

So basically

class Places365Tester(TestCase):
    pass

Yes, something like

class TesterBase(TestCase):
    # base implementation for tester
    def generic_classification_dataset_test(...)
    ...

class Tester(TesterBase):  # find better name
    # all datasets tests that are implemented as a single function
    def test_imagefolder(...):
    ...

class Places365Tester(TesterBase):
    ...

Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot!

@fmassa fmassa merged commit fc69c22 into pytorch:master Aug 25, 2020
@pmeier pmeier deleted the places365 branch August 25, 2020 17:12
@pmeier pmeier mentioned this pull request Aug 27, 2020
bryant1410 pushed a commit to bryant1410/vision-1 that referenced this pull request Nov 22, 2020
* initial draft

* [dirty] progress

* remove inheritance from ImageFolder

* add tests

* lint

* fix type hints

* align getitem with other datasets

* remove unused import

* add docstring

* guard existing image folders from overwrite

* add missing entry in docstring

* make fixpath more legible

* add Places365 to docs
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

Successfully merging this pull request may close these issues.

2 participants