Skip to content

Fix Fat dataset loader - #312

Open
amine789 wants to merge 5 commits into
oarriaga:masterfrom
amine789:fat
Open

Fix Fat dataset loader#312
amine789 wants to merge 5 commits into
oarriaga:masterfrom
amine789:fat

Conversation

@amine789

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread paz/datasets/fat.py
Comment on lines -8 to -9
from ..abstract import Loader
from .utils import get_class_names

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

please keep the previous conventions of importing from inside paz

Comment thread paz/datasets/fat.py Outdated
"""
# TODO: Allow selection of class_names.
def __init__(self, path, split='train', class_names='all'):
def __init__(self, path, split=(0.4, 0.4, 0.2), class_names = 'all'):

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

the variable split is only intended to set to be 'train', 'validation' or 'test'. Please don't change that behavior.

Comment thread paz/datasets/fat.py Outdated
Comment on lines +68 to +77
data_dict = {}
train_split = int(len(self.data) * self.split[0])
test_split = int(len(self.data) * self.split[1])
validation_split = int(len(self.data) * self.split[2])
data_dict['train'] = self.data[:train_split]
data_dict['test'] = self.data[
train_split:(train_split + test_split)]
data_dict['validation'] = self.data[(train_split + test_split):]
return data_dict

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

One should instantiate a single data loader per dataset.

Comment thread paz/datasets/fat.py Outdated
Comment on lines +108 to +110

fat = Fat('/home/ramit/git/paz')

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this seems to not belong here.

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.

2 participants