-
Notifications
You must be signed in to change notification settings - Fork 72
Metric interface clean up #983
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #983 +/- ##
==========================================
+ Coverage 88.96% 89.14% +0.17%
==========================================
Files 218 218
Lines 15089 15012 -77
==========================================
- Hits 13424 13382 -42
+ Misses 1665 1630 -35 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
|
|
||
| class GiniBase(DecomposedMetric, RankingMetricBase): | ||
| class GiniBase(RankingMetricBase): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does RankingMetricBase inherit from ListMetric? Gini is not a listwise metric, so if it does then we should either:
- Make
RankingMetricBaseno longer inherit fromListMetric, and update all the ranking metrics that are listwise metrics to inherit from bothRankingMetricBaseandListMetric. - Rename
RankingMetricBasetoRankingListMetricBaseand makeGiniBaseno longer inherit fromRankingMetricBase - A combination of the two: introduce
RankingListMetricBaseto be both ranking metric and list metric, and make the other metrics inherit from it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RankingMetricBase inherits from Metric.
As we discussed, I --
DecomposedMetricclass,measure_runmethod inGlobalMetricclass, and avoided extendingMetricclass.