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.
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
Unify template and quality metrics #3537
Unify template and quality metrics #3537
Changes from 10 commits
530864b
908318a
2706a0b
2246071
3579934
66190c3
2de25b4
8f66024
fdc01f5
9db0b83
039b408
771de98
de7210a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
a deep copy is expensive, but there's no way around it I guess right?
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.
Yeah, this is the backwards compatibility case, where the individual
metric_kwargs
dict (which used to be forced to be the same for every template metric) gets copied repeatedly intometric_params
. If there isn't a deepcopy,metric_params
is just a dict of references to themetric_kwargs
dict. So if one gets altered, they all do. Luckily, the dict is just a dict of kwargs (so is small) and this code only runs on instantiation if someone tries to use the old notation. So shouldn't be a performance issue.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.
So for templates we don't have the same structure as for quality metrics where we have a separate py file with all these names and a list of metrics?
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.
Yeah, it's a bit different. Here we're copying the
qm_compute_name_to_column_names
dict, which is the important dict for getting the actual dataframe column names. The other stuff inquality_metric_list.py
is mostly used for the pca/non-pca split. We could follow the quality_metrics structure and make atemplatemetrics
folder insidesrc/spikeinterface/postprocessing
?? @alejoe91 thoughts?