-
Notifications
You must be signed in to change notification settings - Fork 58
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
Add Workflow and CBMAWorkflow classes. Support pairwise CBMA workflows #809
Merged
+765
−415
Merged
Changes from 8 commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
cb1fa66
Convert cbma_workflow into a class. Support pairwise estimators
JulioAPeraza 72e1ef4
Add support for reports and diagnostics with pairwise estimator
JulioAPeraza adfb9f1
fix dicstring
JulioAPeraza 48b5aed
add deprecation warning
JulioAPeraza 5ffa968
fix test for tables == None
JulioAPeraza 7a40379
add version changes to docstring
JulioAPeraza 73fc15e
fix documentation
JulioAPeraza 8839cf6
improve coverage
JulioAPeraza 9b77e9b
Use specificity maps instead
JulioAPeraza 9fb98cc
Merge _preprocess_input
JulioAPeraza 5455231
remove Workflow from __init__
JulioAPeraza ebd4b13
Add Pairwise estimator report
JulioAPeraza 56ec484
update diagnostics
JulioAPeraza 78bcb0e
reduce the number of iterations. we are running out of time/memory in…
JulioAPeraza 1d25b02
Merge branch 'neurostuff:main' into enh-workflow
JulioAPeraza 2268f17
see if using focuscounter reduces the time for building the documenta…
JulioAPeraza c7a7bf8
New parameter display_second_group
JulioAPeraza b08e712
Update 08_plot_cbma_subtraction_conjunction.py
JulioAPeraza a3573af
Add dataset 2 to summary
JulioAPeraza 6c50469
Update diagnostics.py
JulioAPeraza 3447502
Update versionchanged
JulioAPeraza 5d33e92
Merge branch 'neurostuff:main' into enh-workflow
JulioAPeraza 5630c9f
Reorder matrix only if more than 1 cluster/experiment
JulioAPeraza 4f18d84
display_second_group in the example
JulioAPeraza 98bc738
fix #814
JulioAPeraza 9b950e3
Use iframe only for connectome
JulioAPeraza 363effd
Set the size of the heatmap proportional to rows and columns
JulioAPeraza fa5d876
Separate positive from negative tail contribution table for pairwise …
JulioAPeraza 385091d
Add subsubtitle
JulioAPeraza 761ee12
Merge branch 'main' into enh-workflow
JulioAPeraza b923cd9
Test a realistic scenario with different dset1 and desert 2
JulioAPeraza 73347ad
Apply @jdkent code review
JulioAPeraza e7360ed
consider the length of the study label in the figure size
JulioAPeraza afdf4a4
fix issues with figure sizes
JulioAPeraza fbd7ae4
Define "PositiveTail" and "NegativeTail" as variables
JulioAPeraza a4e8acd
Update diagnostics.py
JulioAPeraza 883fa6e
Make a distinction between studies and experiments in report
JulioAPeraza cc1a3e0
Restore the diagnostics summary
JulioAPeraza 8ac8e50
Limit the colormap to the total number of clusters
JulioAPeraza d46e3ca
Update 08_plot_cbma_subtraction_conjunction.py
JulioAPeraza File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
For MKDAChi2, the z-values can be positive or negative (you can check the z-map results from running the test in this pull request: #811).
I think we would want to display both Positive and Negative Tail cluster with Pairwise estimators.
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.
We are displaying Positive and Negative tail cluster tables and maps. Here, I'm only excluding the Negative tail contribution table, which will be zero for studies in
id1
. I think if we want to display that table we would need to plot it in a separate figure and use the id2 for estimating the values. WDYT?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.
Also in the report, we only show the summary for
id1
andcoordinates1
from dataset1. Should we include another section for the second group?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.
This is a little more questionable, overall, I would say yes, we do want to display
id2
for the coordinates. for the contribution table, this is a bit debatable. With ALESubtraction, yes, I think it is useful to know how both groups contributed to the existing clusters.For the MKDAChi2 I think it is less useful to see a contribution table (and with jacknife it would be computationally prohibitive if one was using the entire neurosynth dataset).
But the notion that ALESubtraction is used for more similarly sized groups and MKDAChi2 is used for comparing a relatively smaller dataset to a huge dataset is only reflective of how we are currently the algorithms, there's nothing stopping you from using ALESubtraction with neurosynth.
So I think the answer would be creating an option/flag for the user to decide whether they want to show the second group or not. could be called
display_second_group
or something, with a default value ofFalse
.