Conversation
|
Please set a versioning label of either |
There was a problem hiding this comment.
Pull Request Overview
This PR implements improvements to projection matrix calculations by adding new functionality for cortical parent-level masking and reorganizing code structure. The changes enhance the flexibility of projection analysis by allowing mask methods to be applied at aggregated cortical regions rather than individual layers.
- Adds a new
apply_mask_at_cortical_parent_levelparameter to enable masking at cortical parent structures - Moves projection normalization function to a shared utility module for better code organization
- Updates executable scripts to support the new masking functionality and improves parameter handling
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| setup.cfg | Adds console script entry point for distance calculation utility |
| morph_utils/proj_mat_utils.py | New utility module containing projection matrix functions including normalization and roll-up operations |
| morph_utils/executable_scripts/projection_matrix_from_swc_directory.py | Updates to support cortical parent-level masking and imports refactored normalization function |
| morph_utils/executable_scripts/projection_matrix_for_single_cell.py | Adds cortical parent-level masking support and improves mask method validation |
| morph_utils/executable_scripts/distance_between_nodes_for_directory.py | Changes default compartment types and adds debug output |
| morph_utils/ccf.py | Implements core cortical parent-level masking logic and adds de_layer function |
| morph_utils_aggregate_single_cell_projs = morph_utils.executable_scripts.aggregate_single_cell_projection_csvs:console_script | ||
| morph_utils_move_somas_left_hemisphere = morph_utils.executable_scripts.move_somas_to_left_hemisphere_swc_directory:console_script | ||
| morph_utils_local_crop_ccf_swcs = morph_utils.executable_scripts.local_crop_ccf_swc_directory:console_script | ||
| morph_utils_dsit_btwn_nodes_directory = morph_utils.executable_scripts.distance_between_nodes_for_directory:console_script No newline at end of file |
There was a problem hiding this comment.
There is a spelling error in the console script name. 'dsit' should be 'dist'.
Suggested change
| morph_utils_dsit_btwn_nodes_directory = morph_utils.executable_scripts.distance_between_nodes_for_directory:console_script | |
| morph_utils_dist_btwn_nodes_directory = morph_utils.executable_scripts.distance_between_nodes_for_directory:console_script |
| ccf_swc_directory = ags.fields.InputDir(description='directory with micron resolution ccf registered files') | ||
| output_directory = ags.fields.OutputDir(description="output directory") | ||
| mask_method = ags.fields.Str(description = " 'tip_and_branch', 'branch', 'tip', or 'tip_or_branch' ") | ||
| apply_mask_at_cortical_parent_level = ags.fields.Bool( descriptions='If True, the `mask_method` will be applied at aggregated cortical regions') |
There was a problem hiding this comment.
The parameter name should be 'description' not 'descriptions'.
Suggested change
| apply_mask_at_cortical_parent_level = ags.fields.Bool( descriptions='If True, the `mask_method` will be applied at aggregated cortical regions') | |
| apply_mask_at_cortical_parent_level = ags.fields.Bool( description='If True, the `mask_method` will be applied at aggregated cortical regions') |
| input_swc_file = ags.fields.InputFile(description='directory with micron resolution ccf registered files') | ||
| output_projection_csv = ags.fields.OutputFile(description="output projection csv") | ||
| mask_method = ags.fields.Str(description = " 'tip_and_branch', 'branch', 'tip', or 'tip_or_branch' ") | ||
| apply_mask_at_cortical_parent_level = ags.fields.Bool( descriptions='If True, the `mask_method` will be applied at aggregated cortical regions') |
There was a problem hiding this comment.
The parameter name should be 'description' not 'descriptions'.
Suggested change
| apply_mask_at_cortical_parent_level = ags.fields.Bool( descriptions='If True, the `mask_method` will be applied at aggregated cortical regions') | |
| apply_mask_at_cortical_parent_level = ags.fields.Bool( description='If True, the `mask_method` will be applied at aggregated cortical regions') |
| @@ -10,7 +10,7 @@ | |||
| class IO_Schema(ags.ArgSchema): | |||
| swc_input_directory = ags.fields.InputDir(description='directory with swc files') | |||
| output_file = ags.fields.OutputFile(descripion='output csv with distances between files') | |||
There was a problem hiding this comment.
The parameter name should be 'description' not 'descripion'.
Suggested change
| output_file = ags.fields.OutputFile(descripion='output csv with distances between files') | |
| output_file = ags.fields.OutputFile(description='output csv with distances between files') |
| branch nodes. | ||
| apply_mask_at_cortical_parent_level (bool): If True, the `mask_method` will be applied to aggregated cortical | ||
| regions. E.g. if `mask_method`='tip_and_branch' and apply_mask_at_cortical_parent_level = True, then | ||
| the tip-and-branch mask will be enforced at the (e.g.) VISp level, instead of in VISp1, VISp2/3 etc. independantly |
There was a problem hiding this comment.
The word 'independantly' should be spelled 'independently'.
Suggested change
| the tip-and-branch mask will be enforced at the (e.g.) VISp level, instead of in VISp1, VISp2/3 etc. independantly | |
| the tip-and-branch mask will be enforced at the (e.g.) VISp level, instead of in VISp1, VISp2/3 etc. independently |
| morph_df['ccf_structure_rollup'] = morph_df['ccf_structure'].map(de_layer) | ||
| morph_df["ccf_structure_sided_rollup"] = morph_df.apply(lambda row: "ipsi_{}".format(row.ccf_structure_rollup) if row.z<z_midline else "contra_{}".format(row.ccf_structure_rollup), axis=1) | ||
|
|
||
| # mask the morphology dataframe accordingly |
There was a problem hiding this comment.
[nitpick] The word 'accordinagly' in the original comment was corrected to 'accordingly', but there's a trailing space that should be removed.
Suggested change
| # mask the morphology dataframe accordingly | |
| # mask the morphology dataframe accordingly |
| class IO_Schema(ags.ArgSchema): | ||
| ccf_swc_directory = ags.fields.InputDir(description='directory with micron resolution ccf registered files') | ||
| output_directory = ags.fields.OutputDir(description="output directory") | ||
| mask_method = ags.fields.Str(description = " 'tip_and_branch', 'branch', 'tip', or 'tip_or_branch' ") |
There was a problem hiding this comment.
The mask_method field is now required (no default value) but it was previously optional with a default. This could be a breaking change for existing code that doesn't provide this parameter.
Suggested change
| mask_method = ags.fields.Str(description = " 'tip_and_branch', 'branch', 'tip', or 'tip_or_branch' ") | |
| mask_method = ags.fields.Str(default='tip_and_branch', description = " 'tip_and_branch', 'branch', 'tip', or 'tip_or_branch' ") |
| class IO_Schema(ags.ArgSchema): | ||
| input_swc_file = ags.fields.InputFile(description='directory with micron resolution ccf registered files') | ||
| output_projection_csv = ags.fields.OutputFile(description="output projection csv") | ||
| mask_method = ags.fields.Str(description = " 'tip_and_branch', 'branch', 'tip', or 'tip_or_branch' ") |
There was a problem hiding this comment.
The mask_method field is now required (no default value) but it was previously optional with a default. This could be a breaking change for existing code that doesn't provide this parameter.
Suggested change
| mask_method = ags.fields.Str(description = " 'tip_and_branch', 'branch', 'tip', or 'tip_or_branch' ") | |
| mask_method = ags.fields.Str(default='tip_and_branch', description = " 'tip_and_branch', 'branch', 'tip', or 'tip_or_branch' ") |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
No description provided.