-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Derived Harmonized Volumes in Age Trends module #167
Conversation
53828a9
to
dc57ff8
Compare
@AbdulkadirA There was actually another file that needed to be added to the repository that had the mappings of single volumes that make up derived volumes. I added this file to the scripts by making a series of functions like with MUSE_ROI_Dictionary.csv |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@melhemr Thanks for the update. I recommend to name the variables that relate to the mapping of single to derived volumes as mapping
not dictionary
. The derived volumes still appear in the harmonization module. After loading and applying a harmonization model, the variable (MUSE) GM
, for instance, can be selected which leads to the app crashing.
1f750e5
to
2d33c3a
Compare
@AbdulkadirA I made so that in the harmonization module, if a DERIVED ROI is selected, it sets the combo box to Right Anterior Cingulate gyrus (the SINGLE ROI that always comes up first), and resets the plot to that ROI as well. I also changed the name of the single -> derived mapping file to DerivedMUSEMap, and all functions are similarly changed. The harmonized SINGLE ROI residuals are produced, and the DERIVED ROIs are generated and can be plotted in Age trends. |
2d33c3a
to
91eb9cf
Compare
@AbdulkadirA I was able to fix the issue with the sum for derived volumes, as well are making sure the combobox only includes SINGLE ROIs, but I have been struggling to fix the problem of what to do if the input in the combobox is not valid. I've tried a few ways, particularly the one I've pushed here, but nothing has worked. Any suggestions? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@melhemr See suggestion below.
91eb9cf
to
84ad1b4
Compare
84ad1b4
to
8d43225
Compare
@AbdulkadirA I added code to fix #170 as well |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With pandas==1.3.4
it works. However, at some point the mapping of derived ROIs is printed in the terminal. This is not necessary. Otherwise good to merge.
8d43225
to
6429b39
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@melhemr Thanks. The derived volumes are calculated and can be added to the data frame.
No description provided.