[AL-4149] Validate metric names against reserved names #770
Conversation
labelbox/data/metrics/iou/iou.py
Outdated
| continue | ||
| metrics.append( | ||
| ScalarMetric(metric_name="iou", feature_name=key, value=value)) | ||
| ScalarMetric(metric_name="custom_iou", feature_name=key, value=value)) |
There was a problem hiding this comment.
as we agreed in Slack discussion, we rename these to custom_iou
| 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 |
There was a problem hiding this comment.
I think we shouldn't force all lowercase, it's just for validation purpose
There was a problem hiding this comment.
I think we should be consistent, if we're validating against lower-cased value then we should store it that way as well
There was a problem hiding this comment.
but on the other hand it will break existing metrics, yeah 🤔
There was a problem hiding this comment.
I'd say validating against lower-cased metric name feels weird, we should just trim it
There was a problem hiding this comment.
alright, I think that's not so important
now we return the source name as it's currently in prod version 👌
| @validator('metric_name') | ||
| def validate_metric_name(cls, name: str): | ||
| clean_name = name.lower().strip() | ||
| if name.lower().strip() in RESERVED_METRIC_NAMES: |
There was a problem hiding this comment.
super nit, might want to change name.lower().strip() to clean_name
Validate metric names against the list of reserved ones