Implement handling of bids_id column
#34
Conversation
| @@ -183,7 +183,8 @@ def process_bagel(contents, filename): | |||
| data, total_subjects, sessions, upload_error = util.parse_csv_contents( | |||
| contents=contents, filename=filename | |||
| ) | |||
| except Exception: | |||
| except Exception as exc: | |||
| print(exc) # for debugging | |||
There was a problem hiding this comment.
You can take a look at https://docs.python.org/3/library/logging.html to collect these types of messages and handle them - it gives you more clean control over what the output and where
| @@ -65,13 +64,21 @@ def extract_pipelines(bagel: pd.DataFrame) -> dict: | |||
| return pipelines_dict | |||
|
|
|||
|
|
|||
| def get_id_columns(data: pd.DataFrame) -> list: | |||
| """Returns names of columns which identify a given participant record""" | |||
| return ( | |||
There was a problem hiding this comment.
minor, but: is there a reason to do this over just if ... return; else return? I find that cleaner to read
There was a problem hiding this comment.
Ah, mainly brevity. In some past PRs I've gotten suggestions to switch simple if-else (that is, returning a single object) blocks to ternary operators, so I guess the preference depends on who's reviewing the code 😅. I would like to keep the style consistent across the app, if possible. Would you recommend defaulting to if ... return; else return ?
There was a problem hiding this comment.
I see, I think this is still readable as is. I would not use ternary if this got any longer. Consistency is a good idea, so if you've used this elsewhere, then let's keep it
| @@ -65,13 +64,21 @@ def extract_pipelines(bagel: pd.DataFrame) -> dict: | |||
| return pipelines_dict | |||
|
|
|||
|
|
|||
| def get_id_columns(data: pd.DataFrame) -> list: | |||
| """Returns names of columns which identify a given participant record""" | |||
| return ( | |||
There was a problem hiding this comment.
I see, I think this is still readable as is. I would not use ternary if this got any longer. Consistency is a good idea, so if you've used this elsewhere, then let's keep it
The most up-to-date bagel.csv files generated by mr_proc trackers will have an additional column
bids_id(which will have values for subjects with BIDS imaging data) that is not currently defined in the bagel schema.This PR adds it to the schema as an optional column and updates utility/plotting functions to handle this additional column when constructing the overview page of the dashboard.
Small changes:
example_bagel.csvupdated:bids_idcol added, nonsensicalhas_mri_datastatuses fixedCloses #24