[TEST][REF] Move and test functions related to creating scans.tsv for AIBL #1390
Conversation
There was a problem hiding this comment.
Thanks @AliceJoubert !
I made a first pass with small comments and suggestions.
Looks good to me but I will take a closer look next week.
| write_scans_tsv(bids_path, scans_dict) | ||
|
|
||
|
|
||
| def _flutemeta_badline(line: List[str]) -> Optional[List[str]]: |
There was a problem hiding this comment.
Sometimes I also forget about it, but since we are in the Python 3.9+ world, we can use builtins for type hints. I don't think we need to update all signatures in an industrial way, but slowly make the code evolve to use new typing syntax.
| def _flutemeta_badline(line: List[str]) -> Optional[List[str]]: | |
| def _flutemeta_badline(line: list[str]) -> Optional[list[str]]: |
|
|
||
| def _flutemeta_badline(line: List[str]) -> Optional[List[str]]: | ||
| # Fix for malformed flutemeta file in AIBL (see #796). | ||
| # Some flutemeta lines contain a non-coded string value at the second-to-last position. This value |
There was a problem hiding this comment.
Could you turn this into a docstring ?
| write_scans_tsv(bids_path, scans_dict) | ||
|
|
||
|
|
||
| def _flutemeta_badline(line: List[str]) -> Optional[List[str]]: |
There was a problem hiding this comment.
Could you rename to start with an action verb, _handle_flutemeta_badline maybe ?
| write_sessions_tsv | ||
| """ | ||
|
|
||
| from clinica.iotools.bids_utils import _get_pet_tracer_from_filename |
There was a problem hiding this comment.
Probably means that we need to make this function public...
|
|
||
| supported_modalities = ("anat", "dwi", "func", "pet") | ||
|
|
||
| for sub in scans_dict.keys(): |
There was a problem hiding this comment.
It costs only 4 characters and it helps our brains:
| for sub in scans_dict.keys(): | |
| for subject in scans_dict.keys(): |
| supported_modalities = ("anat", "dwi", "func", "pet") | ||
|
|
||
| for sub in scans_dict.keys(): | ||
| for session in scans_dict[sub].keys(): |
There was a problem hiding this comment.
Same of previous for loop.
| for session in scans_dict[sub].keys(): | |
| for session in scans_dict[sub]: |
| scans_df = pd.DataFrame() | ||
| tsv_file = bids_dir / sub / session / f"{sub}_{session}_scans.tsv" | ||
| tsv_file.unlink(missing_ok=True) | ||
| for mod in (bids_dir / sub / session).glob("*"): |
There was a problem hiding this comment.
Same idea about names and simplify by replacing the following if condition by a list filter:
| for mod in (bids_dir / sub / session).glob("*"): | |
| for modality in [mod for mod in (bids_dir / subject / session).glob("*") if mod.name in supported_modalities]: |
| return scans_dict | ||
|
|
||
|
|
||
| def write_scans_tsv(bids_dir: Path, scans_dict: dict) -> None: |
There was a problem hiding this comment.
Either these are private, or __all__ needs to be updated to reflect the new public objects of this module, right ? It basically depends if you are using these functions outside of this module.
Close #1352, #1353