📝 Add docstrings to revise_NPR_to_CPCA#425
Conversation
Docstrings generation was requested by @Gab-D-G. * #424 (comment) The following files were modified: * `rabies/analysis_pkg/analysis_functions.py` * `rabies/analysis_pkg/analysis_math.py` * `rabies/analysis_pkg/analysis_wf.py` * `rabies/analysis_pkg/cpca/modeling.py` * `rabies/analysis_pkg/cpca/report.py` * `rabies/analysis_pkg/diagnosis_pkg/diagnosis_functions.py` * `rabies/analysis_pkg/diagnosis_pkg/diagnosis_wf.py` * `rabies/analysis_pkg/diagnosis_pkg/interfaces.py` * `rabies/analysis_pkg/main_wf.py` * `rabies/parser.py` * `scripts/generate_graph.py`
|
Important Review skippedCodeRabbit bot authored PR detected. To trigger a single review, invoke the You can disable this status message by setting the 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull request overview
Adds/updates docstrings across the analysis + diagnosis + CPCA code paths to improve inline documentation, following the docstring-generation request in the CPCA migration work.
Changes:
- Added docstrings to workflow initialization functions (analysis, main analysis, diagnosis).
- Added docstrings to CPCA modeling/reporting utilities and analysis/diagnosis helper functions.
- Expanded CLI parser function docstrings to better describe supported stages/options.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
rabies/analysis_pkg/analysis_functions.py |
Adds docstrings to CPCA interface methods. |
rabies/analysis_pkg/analysis_math.py |
Adds a docstring to dual_regression. |
rabies/analysis_pkg/analysis_wf.py |
Adds docstring to analysis workflow initializer. |
rabies/analysis_pkg/cpca/modeling.py |
Adds docstrings to CPCA modeling entry points. |
rabies/analysis_pkg/cpca/report.py |
Adds docstrings to CPCA reporting utilities (but introduces an indentation bug). |
rabies/analysis_pkg/diagnosis_pkg/diagnosis_functions.py |
Adds docstrings to diagnosis feature extraction/plotting helpers. |
rabies/analysis_pkg/diagnosis_pkg/diagnosis_wf.py |
Adds docstrings to diagnosis workflow and nested helper. |
rabies/analysis_pkg/diagnosis_pkg/interfaces.py |
Adds docstring to dataset-level diagnosis interface run method. |
rabies/analysis_pkg/main_wf.py |
Adds docstrings to main analysis workflow and nested packager. |
rabies/parser.py |
Expands docstrings for parser construction/reading and scan QC threshold parsing. |
scripts/generate_graph.py |
Adds docstring to workflow execution helper in the graph-generation script. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def parse_scan_QC_thresholds(opt): | ||
|
|
||
| # we must add "" around each key manually, as they are not encoded from the parser | ||
| """ | ||
| Parse and validate the scan quality control thresholds string for analysis. |
There was a problem hiding this comment.
This added docstring is not actually a docstring because there’s a comment statement before it in the function body. If you want tooling/introspection to pick it up, place the docstring as the first statement in parse_scan_QC_thresholds (move the comment below it).
| # estimate cosine similarity across the columns of X and Y combined | ||
| """ |
There was a problem hiding this comment.
This triple-quoted string won’t be registered as cosine_similarity’s docstring because it follows a comment statement. Move the docstring to be the first statement inside the function (and keep the existing one-line note as part of the docstring or below it).
| # estimate cosine similarity across the columns of X and Y combined | |
| """ | |
| """ | |
| Estimate cosine similarity across the columns of X and Y combined. |
| Returns: | ||
| fig_list (list): List of matplotlib Figure objects, one per prior component. | ||
| """ | ||
| fig_list = [] |
There was a problem hiding this comment.
fig_list = [] is indented far deeper than the rest of the function body, which will raise an IndentationError (or change block structure) at import time. It should be aligned with the docstring and the subsequent for prior_idx in range(...) loop (same indentation level as other top-level statements in plot_report).
| fig_list = [] | |
| fig_list = [] |
| def execute_workflow(args=None): | ||
| # generates the parser CLI and execute the workflow based on specified parameters. | ||
| """ | ||
| Prepare and configure a RABIES workflow based on command-line arguments. | ||
|
|
||
| Parses CLI arguments, validates output directory paths, initializes logging, verifies template installation, checks for incompatible inclusion/exclusion parameters, and sets up the specified workflow stage (preprocess, confound correction, or analysis). Saves the modified CLI options as a pickle file and returns the prepared workflow object. |
There was a problem hiding this comment.
The triple-quoted string here won’t be treated as the function’s docstring because it’s preceded by a comment statement. If you want this to be an actual docstring (available via execute_workflow.doc and picked up by doc tools), move the docstring to be the first statement in the function body (put the existing comment below it).
| def dual_regression(all_IC_vectors, timeseries): | ||
| ### compute dual regression | ||
| ### Here, we adopt an approach where the algorithm should explain the data | ||
| ### as a linear combination of spatial maps. The data itself, is only temporally | ||
| ### detrended, and not spatially centered, which could cause inconsistencies during | ||
| ### linear regression according to https://mandymejia.com/2018/03/29/the-role-of-centering-in-dual-regression/#:~:text=Dual%20regression%20requires%20centering%20across%20time%20and%20space&text=time%20points.,each%20time%20course%20at%20zero | ||
| ### The fMRI timeseries aren't assumed theoretically to be spatially centered, and | ||
| ### this measure would be removing global signal variations which we are interested in. | ||
| ### Thus we prefer to avoid this step here, despite modelling limitations. | ||
| """ | ||
| Perform dual regression to decompose timeseries data into spatial weights and component timecourses using provided independent component vectors. | ||
|
|
There was a problem hiding this comment.
This string literal is not a real docstring because it appears after multiple comment statements inside the function body. To make it a proper docstring, it needs to be the first statement immediately under def dual_regression(...) (move the explanatory comments below the docstring, or incorporate them into the docstring).
Docstrings generation was requested by @Gab-D-G.
The following files were modified:
rabies/analysis_pkg/analysis_functions.pyrabies/analysis_pkg/analysis_math.pyrabies/analysis_pkg/analysis_wf.pyrabies/analysis_pkg/cpca/modeling.pyrabies/analysis_pkg/cpca/report.pyrabies/analysis_pkg/diagnosis_pkg/diagnosis_functions.pyrabies/analysis_pkg/diagnosis_pkg/diagnosis_wf.pyrabies/analysis_pkg/diagnosis_pkg/interfaces.pyrabies/analysis_pkg/main_wf.pyrabies/parser.pyscripts/generate_graph.pyℹ️ Note