Skip to content

Revert "Ported places365 dataset's tests to the new test framework" #3718

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

Merged
merged 1 commit into from
Apr 23, 2021

Conversation

datumbox
Copy link
Contributor

Reverts #3705

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. I'll deal with the failing download tests. Then we can merge the PR again. Cc @prabhat00155

@datumbox datumbox merged commit d419558 into master Apr 23, 2021
@datumbox datumbox deleted the revert-3705-prabhat00155/port_places365 branch April 23, 2021 19:08
@prabhat00155
Copy link
Contributor

I have created this PR: #3719 to remove old tests from test_datasets_download.

@pmeier
Copy link
Collaborator

pmeier commented Apr 23, 2021

Yeah, I need to fix

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:
root, data = places365
datasets.Places365(root, split=split, small=small, download=True)
return make_download_configs(urls_and_md5s, name="Places365")

Will do that and than we can merge your PR again. Sorry, I missed that during the review.

@prabhat00155
Copy link
Contributor

Yeah, I need to fix

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:
root, data = places365
datasets.Places365(root, split=split, small=small, download=True)
return make_download_configs(urls_and_md5s, name="Places365")

Will do that and than we can merge your PR again. Sorry, I missed that during the review.

@pmeier We already test download=True in the new tests, so we don't need these right? #3719 should fix the issue.

prabhat00155 added a commit that referenced this pull request Apr 26, 2021
prabhat00155 added a commit that referenced this pull request Apr 26, 2021
@datumbox datumbox added the bug label Apr 27, 2021
facebook-github-bot pushed a commit that referenced this pull request May 4, 2021
…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
…test framework (#3705)" (#3718)" (#3731)

Summary: This reverts commit d419558.

Reviewed By: NicolasHug

Differential Revision: D28169118

fbshipit-source-id: 64dd4045e47e6f308e229a8a83a2d2ae29378408
@NicolasHug NicolasHug added module: datasets module: tests revert(ed) For reverted PRs, and PRs that revert other PRs labels May 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cla signed module: datasets module: tests revert(ed) For reverted PRs, and PRs that revert other PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants