-
Notifications
You must be signed in to change notification settings - Fork 6
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
Naming the new metrics API #17
Comments
Tagging @adrinjalali @hildeweerts @amueller @MiroDudik |
Question: according to SLEP006 scikit-learn devs decided to refer to the thing we call |
As someone who wasn't super involved in this |
we call that parameter in SLEP6 metadata since it can potentially include non-sample-aligned data, like column metadata and data metadata. I think for the purpose of this work, those extra params are always sample aligned, so I wouldn't mind calling them |
In terms of names, As for the kwarg name, I prefer As for |
I'm going to also open the discussion about Re. class nameI'm not enamored with any of these. Another option is Re. method=minmaxI prefer the keyword
Re. group_min/maxI am fine with renaming for better clarity, but I have some issues with the suggested alternatives. The issue with [Also, while thinking about this, bear in mind that it needs to work in compounds like Re. conditional_featuresI'm not too happy about the name although it came from me based on the terminology conditional statistical parity. But it doesn't quite work in describing features--the features themselves are not conditional, it is their values that are being conditioned on. So some alternative names:
Re. sample_paramsI still find this a bit ambiguous. How about |
Thanks @MiroDudik . I've added those above (and @hildeweerts) |
Class NameIn my opinion method=minmaxI strongly prefer In terms of options, I think it's important to stay consistent. For example, Options for group_min/max()We could just call it I am also not convinced we need Options for conditional featuresNow that Miro said it I cannot unsee the inaccurate terminology, lol. I think proper English would be Sample ParamsI find |
Thanks @hildeweerts . I've put your suggestions up in the main post (although not your reasoning - we can talk about that in the meeting). I'm also not convinced we need all of |
Mhmm... I'd like to suggest tweaks to the above. Re. class nameThe final two contenders were
After discussing this with @hannawallach, I'm now strongly in favor of There's a really really minor usage of "metric frame" terminology in the theory of algebraic structures, but I don't think we need to worry about it: Are there any objections to this? Re.
|
Tagging @hildeweerts and @adrinjalali for comments. I don't have any strong preference, beyond having a decision so that I can finalise the PR. |
I am fine with either What I like about I like |
@hildeweerts : good feedback re. |
Unless @adrinjalali objects I will go back to |
With the new metrics object almost finalised, we need to decide on the names. Currently, the object looks like this:
With the exception of
__init__
all names here are potentially changeable. The ones I think are most 'controversial' are the class name, and themethod=
argument and its accepted values (if others think differently, those can be discussed too). I will update this starting post with suggestions from the discussion to save everyone having to read through the whole thread each time.Options for the class name
Some thoughts
MetricsFrame
Consensus is on this. Use 'metrics' since 'metric' could be an adjective and not a noun.
Options for
method=minmax
There are two components - the name of the argument itself, and its accepted values. The latter could potentially depend on the former.
For the argument name
method=
Consensus is to stick with
method=
For the allowed values:
between_pairs
(replacesminmax
which has other meanings)to_overall
There isn't a huge amount of love for these, but there's a shortage of concise alternatives that don't suffer from similar ambiguity issues.
Options for
group_min/max()
Stick with
group_min()/max()
. They do have value when there are conditional features present.Options for conditional features
conditional_features
other_features
condition_on
split_by
conditioned_on
conditioning_features
(might sound like a shampoo advert)Sample Params
sample_params
Keep as
sample_params
. The most meaningful alternative issample_props
, and that wasn't felt to be a significant difference.The text was updated successfully, but these errors were encountered: