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

CIFAR: permanent 'data' and 'targets' fields #594

Merged
merged 1 commit into from Sep 11, 2018
Merged

Conversation

dizcza
Copy link
Contributor

@dizcza dizcza commented Sep 3, 2018

I should have done it when I fixed MNIST 'data' and 'targets' fields in #578.

I found that people do use dynamic train_data and test_data fields (see here for example), obfuscating their code a lot.

@fmassa
Copy link
Member

fmassa commented Sep 3, 2018

I'm totally ok with merging such changes, but for the next release we need to make sure that we mention that those attributes have been removed, as this is a backwards-incompatible change.

I'm mentioning this as a self-reminder. I'll have a closer look to your PR later

@fmassa fmassa self-requested a review September 3, 2018 15:06
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.

LGTM, thanks!

@fmassa fmassa merged commit fc7911c into pytorch:master Sep 11, 2018
scottclowe added a commit to scottclowe/cifar10-fast that referenced this pull request Mar 22, 2019
In the latest release of torchvision (v0.2.2), the fields of the
CIFAR10 object containing the data were changed from
- both `train_data` and `test_data` to `data`
- both `train_labels` and `test_labels` to `targets`
https://github.com/pytorch/vision/releases/tag/v0.2.2
These changes were made in PR
pytorch/vision#594

Our code is pulling the data out from the old fields, which are
not supported in current and future versions of torchvision.

To ensure we can support both torchvision <=0.2.1 and >=0.2.2,
the fix checks for and uses whichever of these two versions exists
(with a preference for the new naming convention if it is available).
TheCBaH pushed a commit to TheCBaH/cifar10-fast that referenced this pull request May 5, 2019
In the latest release of torchvision (v0.2.2), the fields of the
CIFAR10 object containing the data were changed from
- both `train_data` and `test_data` to `data`
- both `train_labels` and `test_labels` to `targets`
https://github.com/pytorch/vision/releases/tag/v0.2.2
These changes were made in PR
pytorch/vision#594

Our code is pulling the data out from the old fields, which are
not supported in current and future versions of torchvision.

To ensure we can support both torchvision <=0.2.1 and >=0.2.2,
the fix checks for and uses whichever of these two versions exists
(with a preference for the new naming convention if it is available).
@Cipher731
Copy link

Cipher731 commented May 31, 2019

I don't think you left the "mention" properly.
All I get is AttributeError: 'CIFAR10' object has no attribute 'train_data' in 0.2.2.

@fmassa
Copy link
Member

fmassa commented May 31, 2019

@Cipher731 yes, look like I forgot to add the backward-compatibility fixes.

Given that this has been in two releases already, I'm going to be considering that this has been dropped already and won't be included anymore.

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.

None yet

3 participants