Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

Add cortical layers dataset + support for 1D datasets #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

adri-romsor
Copy link
Collaborator

This is the dataset that @lamblin mentioned. I merged the changes to the parallel_loader.py, so that we only have one file. This includes the dataset file cortical_layers.py (for any number of layers) and the parallel_loader.py small modifications to handle the 1D cases.

@lamblin lamblin mentioned this pull request May 31, 2017
@fvisin fvisin changed the title 1D changes Add cortical layers dataset + support for 1D datasets Jun 3, 2017
@fvisin
Copy link
Owner

fvisin commented Jun 3, 2017

Thank you for the PR! I will be traveling in the next 2 weeks, but I will try my best to look into it in a reasonable amount of time!

@fvisin fvisin self-requested a review June 3, 2017 16:31
@fvisin fvisin force-pushed the master branch 2 times, most recently from c6a8d70 to ff0bbfe Compare June 22, 2017 18:30
Copy link
Owner

@fvisin fvisin left a comment

Choose a reason for hiding this comment

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

Sorry for the long delay, I finally found time to review the PR! :)

from matplotlib import cm

from dataset_loaders.parallel_loader import ThreadedDataset
# from parallel_loader_1D import ThreadedDataset_1D
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove the commented line.

floatX = 'float32'

class CorticalLayersDataset(ThreadedDataset):
'''The Cortical Layers dataset consists of a number of 1D intensity profiles
Copy link
Owner

Choose a reason for hiding this comment

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

Please comply with PEP257: add a short description in the first line followed by a blank line.

PEP257:

Multi-line docstrings consist of a summary line just like a one-line docstring, followed by a blank line, followed by a more elaborate description. The summary line may be used by automatic indexing tools; it is important that it fits on one line and is separated from the rest of the docstring by a blank line. The summary line may be on the same line as the opening quotes or on the next line. The entire docstring is indented the same as the quotes at its first line (see example below).

A string in ['train', 'val', 'valid', 'test'], corresponding to
the set to be returned.
split: float32
A float indicanting the fraction of data to be used for training. The
Copy link
Owner

Choose a reason for hiding this comment

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

Typo

A float indicanting the fraction of data to be used for training. The
remaining data will be used for validation.
smooth_or_raw: string
A string in ['raw', 'smooth', 'both'], correspoding to the channels to be
Copy link
Owner

Choose a reason for hiding this comment

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

corresponding

remaining data will be used for validation.
smooth_or_raw: string
A string in ['raw', 'smooth', 'both'], correspoding to the channels to be
returned. Raw are profiles sampled from the raw image, smooth profiles
Copy link
Owner

Choose a reason for hiding this comment

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

Raw --> 'raw'
smooth --> 'smooth'

import numpy as np
from PIL import Image
import re
import warnings
Copy link
Owner

Choose a reason for hiding this comment

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

Remove unused imports. Please run a PEP8 check on the whole file and fix all the errors.

Copy link
Owner

Choose a reason for hiding this comment

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

@pep8speaks suggest diff

elif self.smooth_raw_both == 'both':
ret['data'] = np.concatenate([smooth,raw],axis=2)

ret['data'] = np.expand_dims(ret['data'],axis=2)
Copy link
Owner

Choose a reason for hiding this comment

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

I'd rather change the assert in the parallel_loaders and leave it 3D here.
We could add a is_1D=True attribute to the class (i.e., before the def __init__) and act accordingly in the parallel_loaders here. If you check for getattr(self, 'is_1D') it should be easy to change the asserts. You can also skip the random_transform in that case, cause all the transforms are for 2D data IIRC.

return

def distinct_colours(self, n):
#returns n distinct colours using nipy_spectral colourmap
Copy link
Owner

Choose a reason for hiding this comment

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

Indentation is not a multiple of 4.

@@ -593,7 +593,8 @@ def fetch_from_dataset(self, batch_to_load):
void_label=self.void_labels,
**self.data_augm_kwargs)

if self.set_has_GT and self._void_labels != []:
if self.set_has_GT and self._void_labels != [] or \
max(self._cmap.keys()) > self.non_void_nclasses-1:
Copy link
Owner

Choose a reason for hiding this comment

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

self._cmap is an optional attribute, this will not work whenever it's not set. Also I don't see how the rest of the code can work if self.set_has_GT==False or if self._void_labels == []. What is the condition that you are trying to check here?

seq_x = seq_x.squeeze(2)
seq_y = seq_y.squeeze(2) if self.return_one_hot else \
seq_y.squeeze(1)
raw_data = raw_data.squeeze(2)
Copy link
Owner

Choose a reason for hiding this comment

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

If you modify the parallel_loaders to accept 3D data out of load_sequence you shouldn't need all these modifications.

@fvisin
Copy link
Owner

fvisin commented Jun 23, 2017

@pep8speaks suggest diff

@pep8speaks
Copy link

Here you go with the gist !

You can ask me to create a PR against this branch with those fixes. Simply comment @pep8speaks pep8ify.

@fvisin @adri-romsor

@fvisin
Copy link
Owner

fvisin commented Dec 29, 2017

Hey Adri! Do you have time to close this PR?
Merry Christmas BTW! :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants