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

MNIST and FashionMNIST now have their own 'raw' and 'processed' folders #601

Merged
merged 2 commits into from
Sep 17, 2018
Merged

MNIST and FashionMNIST now have their own 'raw' and 'processed' folders #601

merged 2 commits into from
Sep 17, 2018

Conversation

dizcza
Copy link
Contributor

@dizcza dizcza commented Sep 11, 2018

Addresses #599.

fmassa
fmassa requested changes Sep 13, 2018
View reviewed changes
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.

This overall looks good, thanks!

But this patch makes MNIST Python3 only, so this needs to be changed.

torchvision/datasets/mnist.py Outdated
pass
else:
raise
os.makedirs(self.raw_folder, exist_ok=True)

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

torchvision/datasets/mnist.py Show resolved Hide resolved
@dizcza
Copy link
Contributor Author

dizcza commented Sep 16, 2018

Ready.

fmassa
fmassa requested changes Sep 17, 2018
View reviewed changes
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.

This looks very good, and is almost ready to be merged.
I have a single comment, can you address it so that I can merge this?

Thanks a lot, the code is looking much better!

torchvision/datasets/mnist.py Show resolved Hide resolved
fmassa
fmassa approved these changes Sep 17, 2018
View reviewed changes
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!

@fmassa fmassa merged commit e564f13 into pytorch:master Sep 17, 2018
@soumith soumith mentioned this pull request Sep 17, 2018
Closed
@dizcza dizcza mentioned this pull request Oct 30, 2018
Merged
knakad added a commit to knakad/sagemaker-python-sdk that referenced this pull request Aug 16, 2019
@knakad
change: eliminate dependency on mnist dataset website
3f1e4f2 
This commit will modify the folder structure to avoid needing to
download the datasets and relying on the dataset website from being
available.
This caused a canary to fail (TT-0407832742).

The dependency has existed since Pytorch 1.1 upgrade, as the directory
structure was modified in pytorch/vision#601
@knakad knakad mentioned this pull request Aug 16, 2019
Merged
4 tasks
knakad added a commit to aws/sagemaker-python-sdk that referenced this pull request Aug 16, 2019
@knakad
change: eliminate dependency on mnist dataset website (#986)
cdd56e6 
This commit will modify the folder structure to avoid needing to
download the datasets and relying on the dataset website from being
available.
This caused a canary to fail.

The dependency has existed since Pytorch 1.1 upgrade, as the directory
structure was modified in pytorch/vision#601
rajveerb pushed a commit to rajveerb/vision that referenced this pull request Nov 30, 2023
@TheKanter
fix typo (pytorch#601)
8bf470c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fmassa fmassa fmassa approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dizcza @fmassa