aorsf classification and regression engines #78
Conversation
|
Wonderful, this is a great start! I just approved running GHA—the failures with snap changes are unrelated to this PR and you are welcome to ignore. Could you add some tests along the lines of those you put together for tidymodels/censored#211? Specifically interested in whether we can reproduce the same results using the aorsf interface and parsnip interface. Another next step here would be to add sections on the newly supported modes to parsnip's documentation. You put together the basics in tidymodels/parsnip#828; a follow-up PR would add |
|
If you're interesting in testing this against our CI, I've just approved workflows. They will fail as-is, but merging upstream should resolve any issues not due to this PR. :) I'm assuming I should stay hands-off until a companion PR is made to parsnip, but feel free to holler if you'd appreciate support. |
|
Thank you! I have updated the parsnip docs and opened tidymodels/parsnip#1120 I merged the upstream main branch in and all tests passed locally, with 2 warnings. Sorry, I know I have been pushing things and opening PRs either during weekends or late on Fridays. My work schedule is a little off and I don't have any expectation whatsoever to hear back outside of normal working hours. |
There was a problem hiding this comment.
Looking really strong.
Sorry, I know I have been pushing things and opening PRs either during weekends or late on Fridays. My work schedule is a little off and I don't have any expectation whatsoever to hear back outside of normal working hours.
No need to apologize at all—thanks for the note! I appreciate you putting the effort in to implement this at all. We're on open-source-time on this repo, so no pressure to move on any specific timeline.🤪
tests/testthat/test-aorsf_data.R
Outdated
| # bug-fix for na_action = 'pass' was pushed in this version | ||
| skip_if_not_installed("aorsf", "0.1.4.9001") |
There was a problem hiding this comment.
Out of curiosity, when do you anticipate this release will be out?
There was a problem hiding this comment.
I think CRAN prefers at most 1 update per month, so early June would be a good time since 0.1.4 was submitted around May 1.
There was a problem hiding this comment.
I think I'd be hesitant to send these changes out before those NA action changes are on CRAN. Was starting to gear up for a bonsai release but can sit tight for a couple more weeks so we can add aorsf support in that one!
There was a problem hiding this comment.
Totally makes sense. No rush on my end. =] It's my bad for not checking the classification and regression modules in aorsf more carefully before I released 0.1.4.
prefer expect_no_condition to assert output runs without error Co-authored-by: Simon P. Couch <[email protected]>
omitting curlies can be dangerous Co-authored-by: Simon P. Couch <[email protected]>
avoid duplicate `as_tibble` calls Co-authored-by: Simon P. Couch <[email protected]>
less verbose post processing instruction Co-authored-by: Simon P. Couch <[email protected]>
admittedly does not impart `usethis::use_test()` workflow happiness :/
|
Thanks for all of your work here! Really excited about these additions. I think these PRs are in a good spot to head to |
|
Awesome! Thank you so much for working with me on this. |
|
👋, @simonpcouch, just wanted to let you know |
Adds support for
aorsf(#73).I am uploading
aorsfversion 0.1.4 to CRAN and I don't expect any problems, but I totally understand if you'd prefer we hold off on this PR until the new version is settled on CRAN.bonsaiis an awesome package. Thanks for creating it!