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 a metric print method #455

Merged
merged 7 commits into from
Dec 1, 2023
Merged

add a metric print method #455

merged 7 commits into from
Dec 1, 2023

Conversation

simonpcouch
Copy link
Contributor

Closes #435!

@EmilHvitfeldt
Copy link
Member

As a general comment, i think we should overhaul the printing of metric_set() at the same time 😄

@simonpcouch
Copy link
Contributor Author

Nice! Related to #454, then. How would you like to move this PR forward/not?

I'd be glad to work on the metric set method, too. I guess a one-line-output metric print method would make the metric set print method nice and consistent. :)

@EmilHvitfeldt
Copy link
Member

I think we should add the metric set printing to this PR. we might learn things when updating that, that informs the individual printing method, so keeping them together seems easier

@simonpcouch
Copy link
Contributor Author

I'm thinking this could look like:

roc_auc
#> A probability metric | direction: maximize

demographic_parity(boop)
#> A class metric       | direction: minimize, group-wise on: boop

...with:

metric_set(roc_auc, demographic_parity(boop))
#> A metric set, consisting of:
#> - A probability metric | direction: maximize
#> - A class metric       | direction: minimize, group-wise on: boop

...with cli formatting where needed. Notably, I think it'd be great to link probability metric and class metric to a {.help}-page clarifying what that means, if possible.

Thoughts?

@EmilHvitfeldt
Copy link
Member

I like the look of that!

I would like to have the names of the metrics in the metric set printing.

@simonpcouch simonpcouch marked this pull request as draft November 8, 2023 14:31
@simonpcouch
Copy link
Contributor Author

Spent some time on this yesterday but didn't quite have time to finish—moving on to other things for at least today. Still need to write code for justification (see rlib/cli#229).

@simonpcouch
Copy link
Contributor Author

Currently, output looks like:

library(yardstick)
dem_parity_boop <- demographic_parity(boop)

roc_auc 
#> A probability metric | direction: maximize

dem_parity_boop
#> A class metric | direction: minimize, group-wise on: boop

demographic_parity(boop)
#> A class metric | direction: minimize, group-wise on: boop

metric_set(roc_auc, dem_parity_boop)
#> A metric set, consisting of:
#> - `roc_auc()`, a probability metric   | direction: maximize
#> - `dem_parity_boop()`, a class metric | direction: minimize, group-wise on:
#> boop

metric_set(roc_auc, demographic_parity(boop))
#> A metric set, consisting of:
#> - `roc_auc()`, a probability metric            | direction: maximize
#> - `demographic_parity(boop)()`, a class metric | direction: minimize,
#> group-wise on: boop

Created on 2023-11-27 with reprex v2.0.2

While format.metric_set() does call format.metric, there's a good bit of post-processing that needs to happen to make justification work. From my reading of cli issues, we don't have many justification features to draw from there. I generally do feel like the rows x columns appearance for printing works well for metric sets and miss that structure with my current attempt. I may hand this off to you here, @EmilHvitfeldt, either for review or revision.

@EmilHvitfeldt EmilHvitfeldt marked this pull request as ready for review December 1, 2023 19:17
NEWS.md Outdated Show resolved Hide resolved
@EmilHvitfeldt
Copy link
Member

This all looks good! added a couple of small changes, will merge when CI runs clean

@EmilHvitfeldt EmilHvitfeldt merged commit b9ea40f into main Dec 1, 2023
12 checks passed
@EmilHvitfeldt EmilHvitfeldt deleted the print-435 branch December 1, 2023 19:49
Copy link

This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Printing method for metrics
2 participants