From 8f09793d2a135b5b7018a1177c659d9d6fd86ca2 Mon Sep 17 00:00:00 2001 From: Sergey Dubinin Date: Thu, 17 Nov 2022 18:04:04 +0500 Subject: [PATCH 1/5] AL-4149: Validate metric names against reserved names --- .../data/annotation_types/metrics/scalar.py | 22 ++++++++++++++++++- labelbox/data/metrics/iou/iou.py | 4 ++-- tests/data/annotation_types/test_metrics.py | 12 ++++++++-- 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/labelbox/data/annotation_types/metrics/scalar.py b/labelbox/data/annotation_types/metrics/scalar.py index 2b1c5e5a1..c04b2fa3f 100644 --- a/labelbox/data/annotation_types/metrics/scalar.py +++ b/labelbox/data/annotation_types/metrics/scalar.py @@ -1,7 +1,7 @@ from typing import Dict, Optional, Union from enum import Enum -from pydantic import confloat +from pydantic import confloat, validator from .base import ConfidenceValue, BaseMetric @@ -16,6 +16,18 @@ class ScalarMetricAggregation(Enum): SUM = "SUM" +RESERVED_METRIC_NAMES = ( + 'true_positive_count', + 'false_positive_count', + 'true_negative_count', + 'false_negative_count', + 'precision', + 'recall', + 'f1', + 'iou' +) + + class ScalarMetric(BaseMetric): """ Class representing scalar metrics @@ -28,6 +40,14 @@ class ScalarMetric(BaseMetric): value: Union[ScalarMetricValue, ScalarMetricConfidenceValue] aggregation: ScalarMetricAggregation = ScalarMetricAggregation.ARITHMETIC_MEAN + @validator('metric_name') + def validate_metric_name(cls, name: str): + clean_name = name.lower().strip() + if name.lower().strip() in RESERVED_METRIC_NAMES: + raise ValueError(f"`{clean_name}` is a reserved metric name. " + "Please provide another value for `metric_name`.") + return clean_name + def dict(self, *args, **kwargs): res = super().dict(*args, **kwargs) if res.get('metric_name') is None: diff --git a/labelbox/data/metrics/iou/iou.py b/labelbox/data/metrics/iou/iou.py index 40a87237b..c38e2b60d 100644 --- a/labelbox/data/metrics/iou/iou.py +++ b/labelbox/data/metrics/iou/iou.py @@ -31,7 +31,7 @@ def miou_metric(ground_truths: List[Union[ObjectAnnotation, # If both gt and preds are empty there is no metric if iou is None: return [] - return [ScalarMetric(metric_name="iou", value=iou)] + return [ScalarMetric(metric_name="custom_iou", value=iou)] def feature_miou_metric(ground_truths: List[Union[ObjectAnnotation, @@ -62,7 +62,7 @@ def feature_miou_metric(ground_truths: List[Union[ObjectAnnotation, if value is None: continue metrics.append( - ScalarMetric(metric_name="iou", feature_name=key, value=value)) + ScalarMetric(metric_name="custom_iou", feature_name=key, value=value)) return metrics diff --git a/tests/data/annotation_types/test_metrics.py b/tests/data/annotation_types/test_metrics.py index 4d18f3f03..0d797d1b0 100644 --- a/tests/data/annotation_types/test_metrics.py +++ b/tests/data/annotation_types/test_metrics.py @@ -5,6 +5,7 @@ from labelbox.data.annotation_types.metrics import ConfusionMatrixMetric, ScalarMetric from labelbox.data.annotation_types.collection import LabelList from labelbox.data.annotation_types import ScalarMetric, Label, ImageData +from labelbox.data.annotation_types.metrics.scalar import RESERVED_METRIC_NAMES def test_legacy_scalar_metric(): @@ -56,7 +57,7 @@ def test_legacy_scalar_metric(): ]) def test_custom_scalar_metric(feature_name, subclass_name, aggregation, value): kwargs = {'aggregation': aggregation} if aggregation is not None else {} - metric = ScalarMetric(metric_name="iou", + metric = ScalarMetric(metric_name="custom_iou", value=value, feature_name=feature_name, subclass_name=subclass_name, @@ -80,7 +81,7 @@ def test_custom_scalar_metric(feature_name, subclass_name, aggregation, value): 'value': value, 'metric_name': - 'iou', + 'custom_iou', **({ 'feature_name': feature_name } if feature_name else {}), @@ -192,3 +193,10 @@ def test_invalid_number_of_confidence_scores(): metric_name="too many scores", value={i / 20.: [0, 1, 2, 3] for i in range(20)}) assert "Number of confidence scores must be greater" in str(exc_info.value) + + +@pytest.mark.parametrize("metric_name", RESERVED_METRIC_NAMES) +def test_reserved_names(metric_name: str): + with pytest.raises(ValidationError) as exc_info: + ScalarMetric(metric_name=metric_name, value=0.5) + assert 'is a reserved metric name' in exc_info.value.errors()[0]['msg'] From d3d76bac4973f2716e94de860e31e643aa052cfc Mon Sep 17 00:00:00 2001 From: Sergey Dubinin Date: Thu, 17 Nov 2022 18:08:48 +0500 Subject: [PATCH 2/5] AL-4149: Adjusted formatting --- labelbox/data/annotation_types/metrics/scalar.py | 13 +++---------- labelbox/data/metrics/iou/iou.py | 4 +++- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/labelbox/data/annotation_types/metrics/scalar.py b/labelbox/data/annotation_types/metrics/scalar.py index c04b2fa3f..5d95e828d 100644 --- a/labelbox/data/annotation_types/metrics/scalar.py +++ b/labelbox/data/annotation_types/metrics/scalar.py @@ -16,16 +16,9 @@ class ScalarMetricAggregation(Enum): SUM = "SUM" -RESERVED_METRIC_NAMES = ( - 'true_positive_count', - 'false_positive_count', - 'true_negative_count', - 'false_negative_count', - 'precision', - 'recall', - 'f1', - 'iou' -) +RESERVED_METRIC_NAMES = ('true_positive_count', 'false_positive_count', + 'true_negative_count', 'false_negative_count', + 'precision', 'recall', 'f1', 'iou') class ScalarMetric(BaseMetric): diff --git a/labelbox/data/metrics/iou/iou.py b/labelbox/data/metrics/iou/iou.py index c38e2b60d..357dc5dc9 100644 --- a/labelbox/data/metrics/iou/iou.py +++ b/labelbox/data/metrics/iou/iou.py @@ -62,7 +62,9 @@ def feature_miou_metric(ground_truths: List[Union[ObjectAnnotation, if value is None: continue metrics.append( - ScalarMetric(metric_name="custom_iou", feature_name=key, value=value)) + ScalarMetric(metric_name="custom_iou", + feature_name=key, + value=value)) return metrics From c2271c5ec1105aeffd185ef054ea29a0df32c2ae Mon Sep 17 00:00:00 2001 From: Sergey Dubinin Date: Thu, 17 Nov 2022 19:02:28 +0500 Subject: [PATCH 3/5] AL-4149: Dont lowercase metric name --- labelbox/data/annotation_types/metrics/scalar.py | 2 +- tests/data/assets/ndjson/custom_scalar_import.json | 6 +++--- tests/data/metrics/iou/feature/test_feature_iou.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/labelbox/data/annotation_types/metrics/scalar.py b/labelbox/data/annotation_types/metrics/scalar.py index 5d95e828d..b1725f8c5 100644 --- a/labelbox/data/annotation_types/metrics/scalar.py +++ b/labelbox/data/annotation_types/metrics/scalar.py @@ -39,7 +39,7 @@ def validate_metric_name(cls, name: str): if name.lower().strip() in RESERVED_METRIC_NAMES: raise ValueError(f"`{clean_name}` is a reserved metric name. " "Please provide another value for `metric_name`.") - return clean_name + return name def dict(self, *args, **kwargs): res = super().dict(*args, **kwargs) diff --git a/tests/data/assets/ndjson/custom_scalar_import.json b/tests/data/assets/ndjson/custom_scalar_import.json index 862f9ce92..f54ee9711 100644 --- a/tests/data/assets/ndjson/custom_scalar_import.json +++ b/tests/data/assets/ndjson/custom_scalar_import.json @@ -1,3 +1,3 @@ -[{"uuid" : "a22bbf6e-b2da-4abe-9a11-df84759f7672","dataRow" : {"id": "ckrmdnqj4000007msh9p2a27r"}, "metricValue" : 0.1, "metricName" : "iou", "featureName" : "sample_class", "subclassName" : "sample_subclass", "aggregation" : "SUM"}, - {"uuid" : "a22bbf6e-b2da-4abe-9a11-df84759f7672","dataRow" : {"id": "ckrmdnqj4000007msh9p2a27r"}, "metricValue" : 0.1, "metricName" : "iou", "featureName" : "sample_class", "aggregation" : "SUM"}, - {"uuid" : "a22bbf6e-b2da-4abe-9a11-df84759f7672","dataRow" : {"id": "ckrmdnqj4000007msh9p2a27r"}, "metricValue" : { "0.1" : 0.1, "0.2" : 0.5}, "metricName" : "iou", "aggregation" : "SUM"}] +[{"uuid" : "a22bbf6e-b2da-4abe-9a11-df84759f7672","dataRow" : {"id": "ckrmdnqj4000007msh9p2a27r"}, "metricValue" : 0.1, "metricName" : "custom_iou", "featureName" : "sample_class", "subclassName" : "sample_subclass", "aggregation" : "SUM"}, + {"uuid" : "a22bbf6e-b2da-4abe-9a11-df84759f7672","dataRow" : {"id": "ckrmdnqj4000007msh9p2a27r"}, "metricValue" : 0.1, "metricName" : "custom_iou", "featureName" : "sample_class", "aggregation" : "SUM"}, + {"uuid" : "a22bbf6e-b2da-4abe-9a11-df84759f7672","dataRow" : {"id": "ckrmdnqj4000007msh9p2a27r"}, "metricValue" : { "0.1" : 0.1, "0.2" : 0.5}, "metricName" : "custom_iou", "aggregation" : "SUM"}] diff --git a/tests/data/metrics/iou/feature/test_feature_iou.py b/tests/data/metrics/iou/feature/test_feature_iou.py index 7e8a5e7f7..653e485d1 100644 --- a/tests/data/metrics/iou/feature/test_feature_iou.py +++ b/tests/data/metrics/iou/feature/test_feature_iou.py @@ -13,7 +13,7 @@ def check_iou(pair): assert math.isclose(result[key], pair.expected[key]) for metric in metrics: - assert metric.metric_name == "iou" + assert metric.metric_name == "custom_iou" if len(pair.expected): assert len(one_metrics) From d6a32c0f17bb357c8bb656ac20b0095188fc2291 Mon Sep 17 00:00:00 2001 From: Sergey Dubinin Date: Mon, 28 Nov 2022 18:05:17 +0500 Subject: [PATCH 4/5] AL-4149: Check for None in the validator --- labelbox/data/annotation_types/metrics/scalar.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/labelbox/data/annotation_types/metrics/scalar.py b/labelbox/data/annotation_types/metrics/scalar.py index b1725f8c5..68d716894 100644 --- a/labelbox/data/annotation_types/metrics/scalar.py +++ b/labelbox/data/annotation_types/metrics/scalar.py @@ -34,7 +34,9 @@ class ScalarMetric(BaseMetric): aggregation: ScalarMetricAggregation = ScalarMetricAggregation.ARITHMETIC_MEAN @validator('metric_name') - def validate_metric_name(cls, name: str): + def validate_metric_name(cls, name: Union[str, None]): + if name is None: + return None clean_name = name.lower().strip() if name.lower().strip() in RESERVED_METRIC_NAMES: raise ValueError(f"`{clean_name}` is a reserved metric name. " From 2ed028e15601386a6ad5c9f3e41133dcde65c154 Mon Sep 17 00:00:00 2001 From: Sergey Dubinin Date: Mon, 28 Nov 2022 18:06:48 +0500 Subject: [PATCH 5/5] AL-4149: CR comment --- labelbox/data/annotation_types/metrics/scalar.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/labelbox/data/annotation_types/metrics/scalar.py b/labelbox/data/annotation_types/metrics/scalar.py index 68d716894..fe93849b6 100644 --- a/labelbox/data/annotation_types/metrics/scalar.py +++ b/labelbox/data/annotation_types/metrics/scalar.py @@ -38,7 +38,7 @@ def validate_metric_name(cls, name: Union[str, None]): if name is None: return None clean_name = name.lower().strip() - if name.lower().strip() in RESERVED_METRIC_NAMES: + if clean_name in RESERVED_METRIC_NAMES: raise ValueError(f"`{clean_name}` is a reserved metric name. " "Please provide another value for `metric_name`.") return name