Start next stage #672

laemtl · 2021-08-27T04:32:05Z

This PR adds support for a -n flag for bids_import to start the visit stage.
python3 bids_import.py -d '/test/' -p loris_config.py -csvn

This implementation uses a basic API class to make use of the next stage API endpoint (see #470)
An API config data is required in dicom-archive/.loris-mri/loris_config.py with the following content:

api = {
    'host'     : '',
    'version'  : 'v0.0.4-dev',
    'username' : '',
    'password' : '',
}

xlecours · 2021-09-03T19:42:20Z

python/bids_import.py

+                scan_data.to_df()
+                for scan_data in bids_reader.bids_layout.get_collections('session', 'scans')
+            ])
+            visit_date = min(scans_data['acq_time'])


In case scans_data['acq_time'] would be not defined, you could use the get function.
ex:
scans_data.get('acq_time', datetime.datetime.now())

not sure I would use time of upload as acq_time here since a scan can be uploaded at a different date than the upload date. Might be best keeping the acquisition time as undef is that case instead of setting a default date. Unless I am misunderstanding your suggestion @xlecours?

oooh, reading further I am starting to guess that you need a visit date to start a visit. In that case, instead of guessing a date, I would return an error message to the user telling them that they need to provide acquisition dates in the proper BIDS file in order to start the visit. Would that make sense?

I makes perfect sense to have a date explicitly provided. I just don't know how it would work. Could it be grep'ed from the BIDS files?

(my original comment was intended to avoid a Python error when lokking for an undefined key)

I believe this is what is being done in line 210. I guess the date could also be inside a session level file? I have not looked at the BIDS session spec in a while but it could be feasible. @laemtl thoughts?

python/lib/api.py

cmadjar

@laemtl I would tend to put the API config in the database_config file and modify the template file https://github.com/aces/Loris-MRI/blob/main/dicom-archive/database_config_template.py to keep all python related configs in the same file. It will also make the install easier.

Note: if we keep a separate file for the API config, it needs to be taken care of during the install process and a template needs to be provided.

laemtl · 2021-09-09T14:50:27Z

@laemtl I would tend to put the API config in the database_config file and modify the template file https://github.com/aces/Loris-MRI/blob/main/dicom-archive/database_config_template.py to keep all python related configs in the same file. It will also make the install easier.

Note: if we keep a separate file for the API config, it needs to be taken care of during the install process and a template needs to be provided.

It is a good option but we should probably rename that file then? loris_config?

cmadjar · 2021-09-09T16:33:56Z

It is a good option but we should probably rename that file then? loris_config?

@laemtl agreed. Initially, it only contained database credentials but it is becoming more of a general config file now. Could be named config.py or loris_python_config.py to make it clear that this config is only for the python scripts? Although, technically the .py extension sort of implies python...

laemtl · 2021-10-19T07:15:08Z

@cmadjar Ready.

cmadjar · 2021-10-22T13:27:58Z

Added the blocked label as it needs this PR to be merged on the LORIS side: aces/Loris#7479

cmadjar · 2021-10-22T13:40:04Z

python/bids_import.py

    """

    # database connection
    db = Database(config_file.mysql, verbose)
    db.connect()

+    # api connection
+    api = Api(config_file.api, verbose)


a check should be added to check whether the api config was set in loris_config.py.

cmadjar · 2021-10-22T14:01:19Z

python/lib/api.py

+            else:
+                self.token = resp_json.get('token')
+        except Exception:
+            print("An error occured. Can't login.")


I tested giving bad credentials and I do not have that error printed on the terminal and it does not exit either.

cmadjar · 2021-10-22T14:02:01Z

python/bids_import.py

-        # stored in the Config module)
+        visit_date = None
+        if startvisit:
+            scans_data = pd.concat([


When I run the script with -n option, I get the follow error originating from here:

warnings.warn("Derivative indexing was requested, but no valid " Traceback (most recent call last): File "/Users/cmadjar/Development/Github/loris-mri/python/bids_import.py", line 513, in <module> main() File "/Users/cmadjar/Development/Github/loris-mri/python/bids_import.py", line 82, in main read_and_insert_bids(bids_dir, config_file, verbose, createcand, createvisit, startvisit) File "/Users/cmadjar/Development/Github/loris-mri/python/bids_import.py", line 212, in read_and_insert_bids scans_data = pd.concat([ File "/Users/cmadjar/Development/python_venv/python_intel_3.9.4/loris-mri-venv/lib/python3.9/site-packages/pandas/core/reshape/concat.py", line 285, in concat op = _Concatenator( File "/Users/cmadjar/Development/python_venv/python_intel_3.9.4/loris-mri-venv/lib/python3.9/site-packages/pandas/core/reshape/concat.py", line 342, in __init__ raise ValueError("No objects to concatenate") ValueError: No objects to concatenate

This was due to the fact I did not have a scans.tsv in my dataset. Might be good to add a check before the if startvisit: to check if proper BIDS files were present.

cmadjar · 2021-10-22T14:19:34Z

python/lib/api.py

+        )
+
+        if (resp.status_code and resp.status_code == 200):
+            print("Next stage successfully started.")


I get Next stage successfully started. but when I look at the information in the session table or the timepoint list module, it does not look like anything happened:

2171 167830 3 1 NULL V1 2 N Not Started NULL NULL NULL NULL NULL NULL NULL Y NULL NULL NULL 2021-10-22 10:14:17 - NULL NULL NULL NULL NULL N NULL NULL false NULL 2172 896706 3 1 NULL V1 2 N Not Started NULL NULL NULL NULL NULL NULL NULL Y NULL NULL NULL 2021-10-22 10:14:17 - NULL NULL NULL NULL NULL N NULL NULL false NULL

A successful response will return 204 as of aces/Loris@c7a83f7

github-actions · 2022-01-27T00:46:33Z

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 20 days.

laemtl requested review from cmadjar and xlecours August 27, 2021 04:32

laemtl force-pushed the 2021-08-27-start-next-stage branch 3 times, most recently from 5c6c205 to 0df0e53 Compare August 27, 2021 04:43

Start next stage

7d94c0e

laemtl force-pushed the 2021-08-27-start-next-stage branch from 0df0e53 to 7d94c0e Compare August 27, 2021 04:47

xlecours reviewed Sep 3, 2021

View reviewed changes

Python < 3.7 support

3110c35

cmadjar reviewed Sep 9, 2021

View reviewed changes

python/lib/api.py Show resolved Hide resolved

cmadjar requested changes Sep 9, 2021

View reviewed changes

laemtl closed this Sep 9, 2021

laemtl reopened this Sep 9, 2021

cmadjar added this to the 24.0.0 milestone Sep 10, 2021

laemtl added 2 commits September 16, 2021 12:32

Flake fixes

1ce9532

Remove print statements

0508917

laemtl force-pushed the 2021-08-27-start-next-stage branch from 5f1469e to 0508917 Compare September 16, 2021 17:36

laemtl added the Needs Work Before Merging label Sep 16, 2021

laemtl force-pushed the 2021-08-27-start-next-stage branch 8 times, most recently from 43ab615 to 30b057c Compare October 19, 2021 06:24

Refactoring API config with install script

45f6529

Error on missing acq time

2390bd3

laemtl force-pushed the 2021-08-27-start-next-stage branch from 30b057c to 2390bd3 Compare October 19, 2021 06:29

laemtl removed the Needs Work Before Merging label Oct 19, 2021

laemtl requested a review from cmadjar October 19, 2021 07:14

laemtl force-pushed the 2021-08-27-start-next-stage branch from 2ec26a7 to 0186020 Compare October 19, 2021 07:39

cmadjar self-assigned this Oct 19, 2021

laemtl force-pushed the 2021-08-27-start-next-stage branch 4 times, most recently from 35f0e49 to e04c0eb Compare October 21, 2021 16:47

Bugfixes

0673182

laemtl force-pushed the 2021-08-27-start-next-stage branch from e04c0eb to 0673182 Compare October 21, 2021 16:50

cmadjar added A-BIDS Area: BIDS. Issues and pull requests related to BIDS and the BIDS import pipeline Blocked Merge it and you die labels Oct 22, 2021

cmadjar requested changes Oct 22, 2021

View reviewed changes

cmadjar removed this from the 24.0.0 milestone Oct 28, 2021

github-actions bot added the Stale label Jan 27, 2022

github-actions bot closed this Mar 13, 2022

cmadjar added this to the N/A milestone Feb 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Start next stage #672

Start next stage #672

laemtl commented Aug 27, 2021 •

edited

Loading

xlecours Sep 3, 2021

cmadjar Sep 9, 2021

cmadjar Sep 9, 2021

xlecours Sep 9, 2021 •

edited

Loading

cmadjar Sep 9, 2021

cmadjar left a comment

laemtl commented Sep 9, 2021 •

edited

Loading

cmadjar commented Sep 9, 2021

laemtl commented Oct 19, 2021

cmadjar commented Oct 22, 2021

cmadjar Oct 22, 2021

cmadjar Oct 22, 2021

cmadjar Oct 22, 2021

cmadjar Oct 22, 2021

xlecours Oct 25, 2021

github-actions bot commented Jan 27, 2022

Start next stage #672

Start next stage #672

Conversation

laemtl commented Aug 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xlecours Sep 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmadjar left a comment

Choose a reason for hiding this comment

laemtl commented Sep 9, 2021 • edited Loading

cmadjar commented Sep 9, 2021

laemtl commented Oct 19, 2021

cmadjar commented Oct 22, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 27, 2022

laemtl commented Aug 27, 2021 •

edited

Loading

xlecours Sep 9, 2021 •

edited

Loading

laemtl commented Sep 9, 2021 •

edited

Loading