Improve extension dependency management in SortingAnalyzer #4321
Conversation
| print(f"Error casting column {col}: {e}") | ||
| return metrics_df | ||
| @classmethod | ||
| def get_optional_dependencies(cls, **params): |
There was a problem hiding this comment.
For BaseMetricExtensions, optional dependencies will include all dependencies for metrics. These are not required, because if a a metric doesn't meet its dependencies it's skipped in the _set_params
|
|
||
| # make a copy of extensions | ||
| # note that the copy of extension handle itself the slicing of units when necessary and also the saveing | ||
| sorted_extensions = _sort_extensions_by_dependency(self.extensions) |
There was a problem hiding this comment.
sorting here was not needed, because the compute_several_extensions already does all the checks and sorting required!
| elif any( | ||
| get_extension_class(d).use_nodepipeline | ||
| for d in extension_class.get_all_dependencies(**extension_params) | ||
| if d in sorted_extensions |
There was a problem hiding this comment.
any extension whose dependencies use node pipeline are moved to the end
| for dependency in required_dependencies: | ||
| if "|" in dependency: | ||
| for dep in dependency.split("|"): | ||
| if extension not in _extension_children[dep]: | ||
| print(f"Adding missing dependency child link: {dep} -> {extension}") | ||
| _extension_children[dep].append(extension) | ||
| else: | ||
| if extension not in _extension_children[dependency]: | ||
| print(f"Adding missing dependency child link: {dependency} -> {extension}") | ||
| _extension_children[dependency].append(extension) |
There was a problem hiding this comment.
If an extension is not a child of another extension by default, but it becomes one based on parameters, we add it to the extension children
| # run fine if dependency computed | ||
| sorting_analyzer.compute(["dummy", "dummy_pipeline"], extension_params=dict(dummy_pipeline=dict(param0=11))) | ||
|
|
||
|
|
There was a problem hiding this comment.
add test: if recompute dummy, dummy_pipeline should be deleted.
|
@samuelgarcia this is good to go! I renamed the function and added the extra test |
1 participant
This PR improves the extension dependency management system.
New class methods for the base
AnalyzerExtensiongive more flexibility, by allowing dependencies to be specified at runtime based on the extension params:The
compute_several_extensionand_sort_extensions_by_dependencywill only use the required extension.Optional extensions are used though to further organize the computation as follows.
Extensions are split into 3 groups:
run_node_pipelineThis new organization allows us to remove the quality metric hack! In the current state, quality metrics were always run as post pipeline because some of them may depend on spike amplitudes/locations. Now, depending on the metric list, quality metrics (and other extensions!) will be moved post pipeline if needed:
See this in effect with some debug prints:
