New caps dataset #562
New caps dataset #562
Conversation
|
Hello @HuguesRoy! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2024-05-15 09:52:28 UTC |
There was a problem hiding this comment.
Thanks a lot @HuguesRoy! Could you please try to write the docstrings with the template I gave you?
clinicadl/utils/caps_dataset/caps_dataset_refactoring/caps_dataset.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Thanks @HuguesRoy !
I had a quick look and made some suggestions on caps_dataset.py, mainly on type hints.
Let me know if some things are not clear.
| def elem_index(self): | ||
| pass | ||
|
|
||
| def label_fn(self, target: Union[str, float, int]) -> Union[float, int]: |
There was a problem hiding this comment.
It seems like the method can return None:
| def label_fn(self, target: Union[str, float, int]) -> Union[float, int]: | |
| def label_fn(self, target: Union[str, float, int]) -> Optional[Union[float, int]]: |
| label: value of the label usable in criterion. | ||
| """ | ||
| domain_code = {"t1": 0, "flair": 1} | ||
| return domain_code[str(target)] |
There was a problem hiding this comment.
Can really target be a float, an integer, or a string ? From the dict above, it seems it can only take two values: "t1" or "flair".
| else: | ||
| return self.label_code[str(target)] | ||
|
|
||
| def domain_fn(self, target: Union[str, float, int]) -> Union[float, int]: |
There was a problem hiding this comment.
Might be missing things, but it seems it can only return integers, right ?
| def domain_fn(self, target: Union[str, float, int]) -> Union[float, int]: | |
| def domain_fn(self, target: Union[str, float, int]) -> int: |
| Args: | ||
| idx: row number of the meta-data contained in self.df | ||
| Returns: | ||
| dictionary with following items: |
There was a problem hiding this comment.
If all subclasses return dictionaries with the same structure, it might be valuable to build a custom type for this instead of relying on dictionaries.
@abc.abstractmethod
def __getitem__(self, idx: int) -> Sample:
....
@dataclass
class Sample:
image: torch.Tensor
label: int | float
participant_id: str
...WDYT?
clinicadl/utils/caps_dataset/caps_dataset_refactoring/caps_dataset.py
Outdated
Show resolved
Hide resolved
|
|
||
|
|
||
| ################################## | ||
| # Transformations |
There was a problem hiding this comment.
Could be worth putting these in a separate module as this file is already pretty large. WDYT ?
|
|
||
|
|
||
| ################################ | ||
| # TSV files loaders |
There was a problem hiding this comment.
Same kind of comment here
| ################################ | ||
| # TSV files loaders | ||
| ################################ | ||
| def load_data_test(test_path: Path, diagnoses_list, baseline=True, multi_cohort=False): |
There was a problem hiding this comment.
| def load_data_test(test_path: Path, diagnoses_list, baseline=True, multi_cohort=False): | |
| def load_data_test(test_path: Path, diagnoses_list: list[str], baseline: bool = True, multi_cohort: bool = False) -> pd.DataFrame: |
| raise ClinicaDLArgumentError( | ||
| "If multi_cohort is given, the TSV_DIRECTORY argument should be a path to a TSV file." | ||
| ) | ||
| else: |
There was a problem hiding this comment.
You don't need to have an else statement here since you raise in the if.
This would spare one level of indentation for the following code block.
| return test_df | ||
|
|
||
|
|
||
| def load_data_test_single(test_path: Path, diagnoses_list, baseline=True): |
There was a problem hiding this comment.
| def load_data_test_single(test_path: Path, diagnoses_list, baseline=True): | |
| def load_data_test_single(test_path: Path, diagnoses_list: list[str], baseline: bool = True) -> pd.DataFrame: |
…aset.py Co-authored-by: Gensollen <nicolas.gensollen@gmail.com>
…aset.py Co-authored-by: thibaultdvx <154365476+thibaultdvx@users.noreply.github.com>
…aset.py Co-authored-by: Gensollen <nicolas.gensollen@gmail.com>
broken path solved
Creation of a new folder caps_dataset_refactoring containing two .py files:
caps_dataset.py: low level class caps-dataset (work only for Image datasets)concat_dataset.py: classes for concatenation, stacking and creation of dataset from control fileFor the moment, these files are not connected to the rest of
clinicadl.