Skip to content

Add more categorical metrics and fix the argument 'dims' renames as 'dim' of the xarray.dot #74

Open
LQscience wants to merge 3 commits intogoogle-research:mainfrom
LQscience:Add_more_categorical_metrics
Open

Add more categorical metrics and fix the argument 'dims' renames as 'dim' of the xarray.dot #74
LQscience wants to merge 3 commits intogoogle-research:mainfrom
LQscience:Add_more_categorical_metrics

Conversation

@LQscience
Copy link
Contributor

  1. Add more categorical metrics and their tests.

modified: weatherbenchX/metrics/categorical.py
modified: weatherbenchX/metrics/metrics_test.py

The calculation functions of false alarm rate, true negative rate, Peirce’s skill score, odds ratio skill score and extremal dependence index are added into the 'weatherbenchX/metrics/categorical.py'. In the meantime, their test functions are added into the 'weatherbenchX/metrics/metrics_test.py'

  1. Replace the augument 'dims' with 'dim'.

modified: weatherbenchX/aggregation.py
modified: weatherbenchX/metrics/categorical.py

The dims argument has been renamed to dim, and will be removed in the future. This renaming is taking place throughout xarray over the next few releases.

	modified:   weatherbenchX/metrics/categorical.py
	modified:   weatherbenchX/metrics/metrics_test.py
Copy link
Collaborator

@raspstephan raspstephan left a comment

Choose a reason for hiding this comment

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

This is amazing. Thank you so much!

@raspstephan
Copy link
Collaborator

Sorry for the delay. There will be an update soon that fixes the unit tests. Then we can proceed with merging this.

@raspstephan
Copy link
Collaborator

Sorry for the long wait. There is a conflict now. If you could fix that, we can submit the PR soon.

@LQscience
Copy link
Contributor Author

Sorry for the long wait. There is a conflict now. If you could fix that, we can submit the PR soon.

OK, I am going to handle with this issue.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We will update the xarray requirement to >=2025.07 which should make this change unnecessary since unfortunately this change won't work for us internally.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The updated has now been merged, so the tests should run even without this change.

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