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.

Sign up for GitHub

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

Jump to bottom

Ported places365 dataset's tests to the new test framework #3705

Merged
merged 9 commits into from
Apr 23, 2021
Merged

Ported places365 dataset's tests to the new test framework #3705

merged 9 commits into from
Apr 23, 2021

Conversation

prabhat00155
Copy link
Contributor

Addresses #3531.

pmeier
pmeier reviewed Apr 22, 2021
View reviewed changes
Copy link
Collaborator

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

Hey @prabhat00155, thanks for the PR! A have a few minor comments below.

test/test_datasets.py Show resolved Hide resolved
test/test_datasets.py Outdated Show resolved Hide resolved
torchvision/datasets/places365.py Outdated Show resolved Hide resolved
pmeier
pmeier approved these changes Apr 23, 2021
View reviewed changes
Copy link
Collaborator

@pmeier pmeier 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 @prabhat00155!

test/test_datasets.py Show resolved Hide resolved
@prabhat00155 prabhat00155 merged commit 4b0b332 into pytorch:master Apr 23, 2021
@prabhat00155 prabhat00155 deleted the prabhat00155/port_places365 branch April 23, 2021 17:11
@datumbox
Copy link
Contributor

datumbox commented Apr 23, 2021 •
edited

It seems that this PR breaks some of our stuff because there are still references on the removed function:

vision/test/test_datasets_download.py

Line 26 in 00c460a

from fakedata_generation import places365_root

vision/test/test_datasets_download.py

Lines 223 to 226 in 00c460a

def places365():
with log_download_attempts(patch=False) as urls_and_md5s:
for split, small in itertools.product(("train-standard", "train-challenge", "val"), (False, True)):
with places365_root(split=split, small=small) as places365:

Confirmed with @pmeier, I'll revert the PR and fix this next week.

datumbox added a commit that referenced this pull request Apr 23, 2021
@datumbox
Revert "Ported places365 dataset's tests to the new test framework (#…
630abef 
…3705)"

This reverts commit 4b0b332.
@datumbox datumbox mentioned this pull request Apr 23, 2021
Merged
datumbox added a commit that referenced this pull request Apr 23, 2021
@datumbox
Revert "Ported places365 dataset's tests to the new test framework (#…
d419558 
…3705)" (#3718)

This reverts commit 4b0b332.
@pmeier pmeier mentioned this pull request Apr 26, 2021
Merged
prabhat00155 added a commit that referenced this pull request Apr 26, 2021
@prabhat00155
Revert "Revert "Ported places365 dataset's tests to the new test fram…
7dd4177 
…ework (#3705)" (#3718)"

This reverts commit d419558.
prabhat00155 added a commit that referenced this pull request Apr 26, 2021
@prabhat00155
Revert "Revert "Ported places365 dataset's tests to the new test fram…
ae63bd0 
…ework (#3705)" (#3718)" (#3731)

This reverts commit d419558.
@pmeier pmeier mentioned this pull request Apr 26, 2021
Closed
facebook-github-bot pushed a commit that referenced this pull request May 4, 2021
@cpuhrsch @pmeier @facebook-github-bot
[fbsync] Ported places365 dataset's tests to the new test framework (#…
33d3bfd 
…3705)

Summary:
* Ported places365 dataset's tests to the new test framework

* Made some attributes private

* Removed unnecessary compute_md5()

* Added test_images_download_preexisting()

Reviewed By: NicolasHug

Differential Revision: D28169137

fbshipit-source-id: 845ea1e6b3efcafd0de69babf60683e555d23364

Co-authored-by: Philip Meier <github.pmeier@posteo.de>
facebook-github-bot pushed a commit that referenced this pull request May 4, 2021
@cpuhrsch @facebook-github-bot
[fbsync] Revert "Ported places365 dataset's tests to the new test fra…
3c27d7d 
…mework (#3705)" (#3718)

Summary: This reverts commit 4b0b332.

Reviewed By: NicolasHug

Differential Revision: D28169132

fbshipit-source-id: 7336a21f89708811eda66ba9ce37319f5f328f89
facebook-github-bot pushed a commit that referenced this pull request May 4, 2021
@cpuhrsch @facebook-github-bot
[fbsync] Revert "Revert "Ported places365 dataset's tests to the new …
452c4e7 
…test framework (#3705)" (#3718)" (#3731)

Summary: This reverts commit d419558.

Reviewed By: NicolasHug

Differential Revision: D28169118

fbshipit-source-id: 64dd4045e47e6f308e229a8a83a2d2ae29378408
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmeier pmeier pmeier approved these changes

@NicolasHug NicolasHug Awaiting requested review from NicolasHug

Assignees
No one assigned
Labels
cla signed enhancement module: datasets module: tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@prabhat00155 @datumbox @pmeier @NicolasHug @facebook-github-bot