Open MiroDudik opened 4 years ago
Good point... I'm creating an issue to address the concern.
Just wondering, have you also considered creating a summary class with methods rather than having separate functions for transformations? I.e. you would call summary.difference()
rather than difference_from_summary(summary)
, a bit like the pandas API. An advantage of this approach is that all available methods can be found at one place in the documentation and can be accessed easily in most IDEs. It also allows for shorter names. And you could create e.g. summary.to_dataframe()
which can be convenient for more novice users who are not as familiar with transforming dictionaries to data frames.
What you are describing is similar to what we had before. We had a long conversation about "summary as an object" route... The main argument against was that the "results" should be simple objects rather than objects with APIs of their own. Maybe @adrinjalali can add.
When it comes to the metrics engine (as appearing in proposal--not all parts are currently implemented), the parts that I like are:
<metric>_<transformation>
conventiongroup_summary(metric_func, ...)
and derived_metric(metric_func, transformation_str, ...)
compositions, where transformation_str
is just 'difference'
, 'ratio'
, etc.make_group_summary(metric_func, ...)
and make_derived_metric(metric_func, transformation_str, ...)
factoriesI am less happy about the mechanism around transformation_func
(e.g., difference_from_summary
) and the matching variants:
derived_metric(metric_group_summary_func, transformation_func, ...)
compositionmake_derived_metric(metric_func, summary_func)
factoryWe should discuss.
I know y'all have a very good understanding of this proposal, but as somebody who's not as familiar with the API as you are, I have a hard time following the proposal. It'd be nice if you could have a motivation, explain the status quo, and explain what the proposed API does and how. It'd make it easier for the rest of us to follow :)
Originally posted by @adrinjalali in https://github.com/fairlearn/fairlearn-proposals/pull/1#issuecomment-598692063