Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MissingCounter metric #520

Merged
merged 3 commits into from
Dec 11, 2024
Merged

Add MissingCounter metric #520

merged 3 commits into from
Dec 11, 2024

Conversation

d-a-bunin
Copy link
Collaborator

@d-a-bunin d-a-bunin commented Dec 9, 2024

Before submitting (must do checklist)

  • Did you read the contribution guide?
  • Did you update the docs? We use Numpy format for all the methods and classes.
  • Did you write any new necessary tests?
  • Did you update the CHANGELOG?

Proposed Changes

See #517.

Closing issues

Closes #517.

@d-a-bunin d-a-bunin self-assigned this Dec 9, 2024
@@ -296,4 +296,55 @@ def wape(y_true: ArrayLike, y_pred: ArrayLike, multioutput: str = "joint") -> Ar
return np.sum(np.abs(y_true_array - y_pred_array), axis=axis) / np.sum(np.abs(y_true_array), axis=axis) # type: ignore


__all__ = ["mae", "mse", "msle", "medae", "r2_score", "mape", "smape", "sign", "max_deviation", "rmse", "wape"]
def count_missing_values(y_true: ArrayLike, y_pred: ArrayLike, multioutput: str = "joint") -> ArrayLike:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I haven't added it into etna/metrics/__init__.py. Do we have to do it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I dont think it is needed there.

Copy link

github-actions bot commented Dec 9, 2024

🚀 Deployed on https://deploy-preview-520--etna-docs.netlify.app

@github-actions github-actions bot temporarily deployed to pull request December 9, 2024 06:48 Inactive
Copy link

codecov bot commented Dec 9, 2024

Codecov Report

Attention: Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.32%. Comparing base (db5257f) to head (5e51c98).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
etna/metrics/functional_metrics.py 85.71% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #520   +/-   ##
=======================================
  Coverage   90.32%   90.32%           
=======================================
  Files         256      256           
  Lines       17199    17214   +15     
=======================================
+ Hits        15535    15549   +14     
- Misses       1664     1665    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@d-a-bunin d-a-bunin requested a review from brsnw250 December 9, 2024 07:43
@@ -296,4 +296,55 @@ def wape(y_true: ArrayLike, y_pred: ArrayLike, multioutput: str = "joint") -> Ar
return np.sum(np.abs(y_true_array - y_pred_array), axis=axis) / np.sum(np.abs(y_true_array), axis=axis) # type: ignore


__all__ = ["mae", "mse", "msle", "medae", "r2_score", "mape", "smape", "sign", "max_deviation", "rmse", "wape"]
def count_missing_values(y_true: ArrayLike, y_pred: ArrayLike, multioutput: str = "joint") -> ArrayLike:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I dont think it is needed there.

@property
def greater_is_better(self) -> None:
"""Whether higher metric value is better."""
return None
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is understandable that we can't control missing values in the true target, but shouldn't it be False? So we prefer more data over the less data.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think metrics should be used to compare different methods with each other on the same dataset. This metric will be the same for the same dataset because it doesn't depend on the model.
So, I'm not really sure what value should it have.

"metric",
(MSE(mode="per-segment", missing_mode="ignore"),),
"metric, expected_type",
((MSE(mode="per-segment", missing_mode="ignore"), type(None)), (MissingCounter(mode="per-segment"), float)),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe NoneType?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is types.NoneType, but it was added in python 3.10: https://docs.python.org/3/library/types.html#types.NoneType

@d-a-bunin d-a-bunin requested a review from brsnw250 December 9, 2024 11:11
brsnw250
brsnw250 previously approved these changes Dec 10, 2024
@github-actions github-actions bot temporarily deployed to pull request December 11, 2024 11:09 Inactive
@d-a-bunin d-a-bunin merged commit 82c1be2 into master Dec 11, 2024
17 checks passed
@d-a-bunin d-a-bunin deleted the issue-517 branch December 11, 2024 12:17
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.

Add MissingCounter metric
2 participants