We recently discussed unifying the template and quality metric extensions. Here’s a proposal for starting that process. They’re already quite similar: the big difference is how they deal with parameters. Quality metrics have a parameter set for each metric, through the qm_params kwarg:
This PR is a proposal to unify the parameter handling of the two metrics extensions, following the more flexible qm_params pattern, calling it metric_params in both cases. In the transition period, if we get the template metrics metrics_kwargs, we propogate these to all metrics. Hence, now the template metrics kwargs would look like:
We also extend the load_params function for ComputeQualityMetric and ComputeTemplateMetric for backwards compatibility and add a get_default_tm_params function to match the existing get_default_qm_params.
This simplifies (and makes some neat refactoring (TODO) possible) for e.g. checking if metrics have already been computed. Would make the code in the curation metrics PR simpler.
Bigger picture, this standardisation is a necessary step if we want to make a more generic MetricExtension class/concept. @alejoe91 discussed a SpikeMetrics class, and it could be a good place for plugins and integration with other projects (e.g. could imagine UnitMatchMetrics class which computes the metrics used in UnitMatch).
Breaking change, so worth discussion. One less disruptive option: keep the metrics_kwargs and qm_params names, but just change the structure.
Begin unifying template and quality metrics.
We recently discussed unifying the template and quality metric extensions. Here’s a proposal for starting that process. They’re already quite similar: the big difference is how they deal with parameters. Quality metrics have a parameter set for each metric, through the
qm_params
kwarg:while the template metrics have one set of parameters shared between all metrics, through the
metrics_kwargs
kwarg:This PR is a proposal to unify the parameter handling of the two metrics extensions, following the more flexible
qm_params
pattern, calling itmetric_params
in both cases. In the transition period, if we get the template metricsmetrics_kwargs
, we propogate these to all metrics. Hence, now the template metrics kwargs would look like:We also extend the
load_params
function forComputeQualityMetric
andComputeTemplateMetric
for backwards compatibility and add aget_default_tm_params
function to match the existingget_default_qm_params
.This simplifies (and makes some neat refactoring (TODO) possible) for e.g. checking if metrics have already been computed. Would make the code in the curation metrics PR simpler.
Bigger picture, this standardisation is a necessary step if we want to make a more generic MetricExtension class/concept. @alejoe91 discussed a SpikeMetrics class, and it could be a good place for plugins and integration with other projects (e.g. could imagine UnitMatchMetrics class which computes the metrics used in UnitMatch).
Breaking change, so worth discussion. One less disruptive option: keep the
metrics_kwargs
andqm_params
names, but just change the structure.