Skip to content

limit the range of numeric predictions#142

Merged
topepo merged 10 commits into
mainfrom
bound-prediction
Apr 4, 2024
Merged

limit the range of numeric predictions#142
topepo merged 10 commits into
mainfrom
bound-prediction

Conversation

@topepo

@topepo topepo commented Apr 4, 2024

Copy link
Copy Markdown
Member

A simple post-processor that can cap the range of predicted values form regression models.

@topepo topepo marked this pull request as ready for review April 4, 2024 16:59
@topepo topepo requested a review from simonpcouch April 4, 2024 16:59

@simonpcouch simonpcouch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looking good😎

In general, I think the arguments to this function would be well-served by the rlang type checkers. There are a good few edge cases (>1-length numeric to upper_limit, non-data-frame x) that would be caught by those utilities, and their messages are quite informative.

Comment thread R/bound_prediction.R Outdated
Comment thread R/bound_prediction.R Outdated
Comment thread R/bound_prediction.R Outdated
Comment thread tests/testthat/test-bound-prediction.R Outdated
Comment thread tests/testthat/test-bound-prediction.R Outdated
library(dplyr)
library(rlang)
data("solubility_test", package = "modeldata")
tune2 <- function() call("tune", "test")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this function ever going to have to deal with tune() values. I guess I'm using my mental model from parsnip here, where upstream packages take care of tune() calls before we send specs off to fit().

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It might erroneously receive that from users. I'd err on the side of over-checking

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added an error for these values (instead of just passing)

Comment thread R/bound_prediction.R Outdated
Comment thread tests/testthat/Rplots.pdf Outdated
Comment thread NEWS.md Outdated
topepo and others added 2 commits April 4, 2024 14:15
Co-authored-by: Simon P. Couch <simonpatrickcouch@gmail.com>
@topepo

topepo commented Apr 4, 2024

Copy link
Copy Markdown
Member Author

I think the arguments to this function would be well-served by the rlang type checkers.

Just added them. This would be a good set of issues for TDD 🥳

topepo and others added 4 commits April 4, 2024 14:35
@topepo topepo merged commit 5aa7ecd into main Apr 4, 2024
@topepo topepo deleted the bound-prediction branch April 4, 2024 19:49
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.

2 participants