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

Disable download for StanfordCars dataset #8309

Merged
merged 5 commits into from
Mar 13, 2024

Conversation

NicolasHug
Copy link
Member

URL has been broken for a long time, so we raise a proper error message pointing to #7545 (comment) now.

I couldn't test this change within our dataset test suite because the default mocks would hard-reset the download parameter to False. So hopefully this is an acceptable test:

In [1]: from torchvision.datasets import StanfordCars
In [3]: StanfordCars(root="/tmp", download=True)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-3-a557e888a29e> in <module>
----> 1 StanfordCars(root="/tmp", download=True)

~/dev/vision/torchvision/datasets/stanford_cars.py in __init__(self, root, split, transform, target_transform, download)
     62 
     63         if download:
---> 64             raise ValueError(
     65                 "The original URL is broken so the StanfordCars dataset is not available for automatic "
     66                 "download anymore. You can try to download it manually following "

ValueError: The original URL is broken so the StanfordCars dataset is not available for automatic download anymore. You can try to download it manually following https://github.com/pytorch/vision/issues/7545#issuecomment-1631441616, and set download=False to avoid this error.

Copy link

pytorch-bot bot commented Mar 12, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/8309

Note: Links to docs will display an error until the docs builds have been completed.

❌ 2 New Failures

As of commit 7e2e132 with merge base 423a1b0 (image):

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

Copy link
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

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

looks good

download (bool, optional): If True, downloads the dataset from the internet and
puts it in root directory. If dataset is already downloaded, it is not
downloaded again."""
download (bool, optional): This parameter exists for backward compatibility but it does not
Copy link
Member Author

Choose a reason for hiding this comment

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

We could deprecate and remove but it would be disruptive for those who explicitly set download=False already. So I'm tempted to keep it around forever.

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.

Two minor things, but nothing blocking. Thanks Nicolas!

torchvision/datasets/stanford_cars.py Outdated Show resolved Hide resolved
torchvision/datasets/stanford_cars.py Show resolved Hide resolved
@NicolasHug
Copy link
Member Author

The tests / download failure is pre-existing and I'm attempting a fix in #8310

The bc job is irrelevant and linux unittests are passing, so this is good to go. Thanks both for the reviews!

@NicolasHug NicolasHug merged commit 6c10d08 into pytorch:main Mar 13, 2024
73 of 75 checks passed
@NicolasHug NicolasHug deleted the stanford_cars branch March 13, 2024 10:17
facebook-github-bot pushed a commit that referenced this pull request Mar 20, 2024
Reviewed By: vmoens

Differential Revision: D55062805

fbshipit-source-id: 7fc8ee2b8aa238c4df057965de46b63ba8a531ae
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: done
Development

Successfully merging this pull request may close these issues.

4 participants