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

240 quantile pivot #241

Merged
merged 23 commits into from
Oct 5, 2023
Merged

240 quantile pivot #241

merged 23 commits into from
Oct 5, 2023

Conversation

dajmcdon
Copy link
Contributor

Closes #240

@dsweber2 This isn't a "layer" but you should be able to call it directly on the predictions and get out something much closer to the submission format.

  • There was already the _wider() version. But naming is better.
  • The _longer() version should be what you want.

This function is a necessary precursor to #237

@dajmcdon dajmcdon requested a review from dsweber2 September 25, 2023 19:52
Copy link
Contributor

@dsweber2 dsweber2 left a comment

Choose a reason for hiding this comment

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

awesome, thanks for this! If it isn't too gnarly, is it possible to change the column names from q and tau to something user defined/default to quantile/value? If not, it's easy enough to pass this through rename after

@dajmcdon
Copy link
Contributor Author

dajmcdon commented Sep 25, 2023

On the q/tau issue, this isn't the only place in the package we have this problem. See also #147 (now 1 year old). We need to standardize these throughout. I'll put it on the Tooling agenda.

Now it's own issue #242.

@dajmcdon
Copy link
Contributor Author

This is a breaking change. Needs a version bump and/or a shim for the dperecated pivot_quantiles().

@dajmcdon dajmcdon changed the base branch from main to v0.0.6 October 1, 2023 20:05
@dajmcdon dajmcdon mentioned this pull request Oct 4, 2023
R/pivot_quantiles.R Outdated Show resolved Hide resolved
R/pivot_quantiles.R Outdated Show resolved Hide resolved
.data <- .data %>%
tidyr::unnest(tidyselect::all_of(col)) %>%
tidyr::pivot_wider(
names_from = "tau", values_from = "q",
Copy link
Contributor

Choose a reason for hiding this comment

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

thought: These names were updated in #243 but since the functions got moved around here vs that branch, merging them will probably get hairy. The #243 changes might not percolate to this new file, so reminder to check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Definitely hairy, unfortunately.

R/pivot_quantiles.R Show resolved Hide resolved
NEWS.md Show resolved Hide resolved
@dajmcdon dajmcdon merged commit b2d1e11 into v0.0.6 Oct 5, 2023
1 check passed
@dajmcdon dajmcdon deleted the 240-quantile-pivot branch October 5, 2023 23:34
@dshemetov dshemetov mentioned this pull request Oct 6, 2023
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.

Refactor quantile pivoting
3 participants