MNIST loader odd implementation

[dizcza]

Wouldn't it be better to change

if self.train:
    self.train_data, self.train_labels = torch.load(
        os.path.join(self.root, self.processed_folder, self.training_file))
else:
    self.test_data, self.test_labels = torch.load(
        os.path.join(self.root, self.processed_folder, self.test_file))

...

if self.train:
    img, target = self.train_data[index], self.train_labels[index]
else:
    img, target = self.test_data[index], self.test_labels[index]

...

def __len__(self):
    if self.train:
        return len(self.train_data)
    else:
        return len(self.test_data)

to

if self.train:
    self.data_file = self.training_file
else:
    self.data_file = self.test_file
self.data, self.labels = torch.load(os.path.join(self.root, self.processed_folder, self.data_file))

...

img, target = self.data[index], self.labels[index]

...

def __len__(self):
    return len(self.data)

Apart from boling down the code complexity, it's not a good practice to have dynamic field names depending on the passed arguments. Benefits are clear for both users and developers who want to subclass MNIST.

torchvision==0.2.1 (same in master)

[fmassa]

Hi,

Thanks for opening this issue.

I totally agree with you that your proposal is better.
I'd be ok with merging such change, even though some people might depend on that particular behavior (but they shouldn't).

Would you be willing to send a PR?

[dizcza]

Yeap. You can close the issue once #578 is merged.