Skip to content

New @metric decorator#505

Merged
ecomodeller merged 6 commits intomainfrom
metric_good_or_bad
Mar 3, 2025
Merged

New @metric decorator#505
ecomodeller merged 6 commits intomainfrom
metric_good_or_bad

Conversation

@ecomodeller
Copy link
Copy Markdown
Member

No description provided.

@ecomodeller ecomodeller marked this pull request as ready for review February 27, 2025 06:47
@ecomodeller ecomodeller changed the title Move large/small to metrics New @metric decorator Feb 27, 2025


def metric(best: str | None = None, has_units: bool = False):
"""Decorator to attach a 'best' attribute to metric functions."""
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update docstring with all 3 functionality

@jsmariegaard
Copy link
Copy Markdown
Member

I wonder if we could use the same decorator concept to handle the aliasing concept in metrics 🤔 right now it is confusing that we have e.g. mae and mean_absolute_error which is the same thing. In a list of available metrics they should not appear as two separate entries and I guess in practice one would never user mean_absolute_error as it would be clumsy in a table. Maybe we could instead have a long_name attribute on mae or display_name on the long one or something.

I guess it would be better to postpone this to a future PR though...

@ecomodeller
Copy link
Copy Markdown
Member Author

ecomodeller commented Feb 27, 2025

TODO:

  • move handling of bias and lin_slope from skill.py to metrics.py
 _one_is_best_metrics = ["lin_slope"]
 _zero_is_best_metrics = ["bias"]

@ecomodeller ecomodeller merged commit 8b5e529 into main Mar 3, 2025
6 checks passed
@ecomodeller ecomodeller deleted the metric_good_or_bad branch March 3, 2025 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants