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 keep_original_cols() to all steps #1167

Merged
merged 4 commits into from Jul 18, 2023
Merged

Conversation

EmilHvitfeldt
Copy link
Member

This PR add keep_original_cols() to all steps that needs it.

Copy link
Contributor

@simonpcouch simonpcouch left a comment

Choose a reason for hiding this comment

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

Love it. :) Thanks for the thorough + nicely patterned test cases.

I had initially been hesitant about the differing defaults but it seems this is already the case with existing steps, so all good there.

Could we run revdeps on this? I wouldn't anticipate that folks would have passed arguments this far back in the formals by position, but just to make sure. It's also otherwise hard for me to eyeball whether these do indeed preserve previous behavior. Pending those checks coming back clean, thumbs up from me!

tests/testthat/_snaps/isomap.md Show resolved Hide resolved
@EmilHvitfeldt
Copy link
Member Author

EmilHvitfeldt commented Jul 18, 2023

I had initially been hesitant about the differing defaults but it seems this is already the case with existing steps, so all good there.

I hate it! but we are doing it to preserve past defaults

They are not able to pass by position past the ..., so it wouldn't be an issue :)

@EmilHvitfeldt
Copy link
Member Author

Revdep looks good! I'll merge

Revdep results:

Main

── CHECK ───────────────────────────────────────────────────────────────────────────────────────────── 63 packages ──
✔ applicable 0.1.0                       ── E: 0     | W: 0     | N: 0additive 0.0.5                         ── E: 0     | W: 0     | N: 0actxps 1.1.0                           ── E: 0     | W: 0     | N: 0autostats 0.4.0                        ── E: 0     | W: 0     | N: 0agua 0.1.3                             ── E: 0     | W: 0     | N: 0baguette 1.0.1                         ── E: 0     | W: 0     | N: 0brulee 0.2.0                           ── E: 0     | W: 0     | N: 0bestNormalize 1.9.0                    ── E: 0     | W: 0     | N: 0bayesian 0.0.9                         ── E: 0     | W: 0     | N: 0card 0.1.0                             ── E: 0     | W: 0     | N: 1bundle 0.1.0                           ── E: 1     | W: 0     | N: 0correlationfunnel 0.2.0                ── E: 0     | W: 0     | N: 1butcher 0.3.2                          ── E: 0     | W: 0     | N: 0CSCNet 0.1.2                           ── E: 0     | W: 0     | N: 0                                               
I D2MCS 1.0.1                            ── E: 1     | W: 0     | N: 0caret 6.0.94                           ── E: 0     | W: 0     | N: 0cvms 1.6.0                             ── E: 0     | W: 0     | N: 0easyalluvial 0.3.1                     ── E: 0     | W: 0     | N: 0finetune 1.1.0                         ── E: 0     | W: 0     | N: 0DALEXtra 2.3.0                         ── E: 0     | W: 0     | N: 0hardhat 1.3.0                          ── E: 0     | W: 0     | N: 0finnts 0.2.4                           ── E: 0     | W: 0     | N: 0embed 1.1.1                            ── E: 0     | W: 0     | N: 1healthcareai 2.5.1                     ── E: 2     | W: 1     | N: 1                                               
I hydrorecipes 0.0.3                     ── E: 1     | W: 0     | N: 0healthyR.ai 0.0.13                     ── E: 0     | W: 0     | N: 0HTRX 1.2.2                             ── E: 0     | W: 0     | N: 1healthyR.ts 0.2.9                      ── E: 0     | W: 0     | N: 1MLDataR 1.0.1                          ── E: 0     | W: 0     | N: 1modelgrid 1.1.1.0                      ── E: 0     | W: 1     | N: 2MachineShop 3.6.2                      ── E: 0     | W: 0     | N: 0modeltime.ensemble 1.0.3               ── E: 0     | W: 0     | N: 1modeltime 1.2.7                        ── E: 0  +1 | W: 0     | N: 1palmerpenguins 0.1.1                   ── E: 0     | W: 0     | N: 0modeltime.resample 0.2.3               ── E: 0     | W: 0     | N: 1nestedmodels 1.0.4                     ── E: 0     | W: 0     | N: 0rsample 1.1.1                          ── E: 0     | W: 0     | N: 0rules 1.0.2                            ── E: 0     | W: 0     | N: 0probably 1.0.2                         ── E: 0     | W: 0     | N: 0shinyrecipes 0.1.0                     ── E: 0     | W: 0     | N: 1sknifedatar 0.1.2                      ── E: 1     | W: 0     | N: 0SomaDataIO 6.0.0                       ── E: 0     | W: 0     | N: 1sparseR 0.2.2                          ── E: 0  +1 | W: 0     | N: 0stabiliser 1.0.6                       ── E: 0     | W: 0     | N: 1stacks 1.0.2                           ── E: 0     | W: 0     | N: 0swag 0.1.0                             ── E: 0     | W: 0     | N: 0tabnet 0.4.0                           ── E: 0     | W: 0     | N: 0text 0.9.99.2                          ── E: 0     | W: 0     | N: 1textrecipes 1.0.3                      ── E: 0     | W: 0     | N: 1tfhub 0.8.1                            ── E: 1     | W: 0     | N: 0themis 1.0.1                           ── E: 0     | W: 0     | N: 0tidybins 0.1.0                         ── E: 0     | W: 0     | N: 1tidyAML 0.0.2                          ── E: 0     | W: 0     | N: 0tidymodels 1.1.0                       ── E: 0     | W: 0     | N: 0tidyclust 0.1.2                        ── E: 0     | W: 0     | N: 0tune 1.1.1                             ── E: 0     | W: 0     | N: 0usemodels 0.2.0                        ── E: 0     | W: 0     | N: 0timetk 2.8.3                           ── E: 0     | W: 0     | N: 1vetiver 0.2.2                          ── E: 0     | W: 0     | N: 0workboots 0.2.0                        ── E: 0     | W: 0     | N: 0workflows 1.1.3                        ── E: 0     | W: 0     | N: 0workflowsets 1.0.1                     ── E: 0     | W: 0     | N: 0waywiser 0.4.1                         ── E: 0     | W: 0     | N: 1                                               
OK: 61                                                                                                             
BROKEN: 2
Total time: 20 min

This branch

── CHECK ───────────────────────────────────────────────────────────────────────────────────────────── 63 packages ──
✔ additive 0.0.5                         ── E: 0     | W: 0     | N: 0agua 0.1.3                             ── E: 0     | W: 0     | N: 0actxps 1.1.0                           ── E: 0     | W: 0     | N: 0applicable 0.1.0                       ── E: 0     | W: 0     | N: 0autostats 0.4.0                        ── E: 0     | W: 0     | N: 0baguette 1.0.1                         ── E: 0     | W: 0     | N: 0brulee 0.2.0                           ── E: 0     | W: 0     | N: 0bestNormalize 1.9.0                    ── E: 0     | W: 0     | N: 0bayesian 0.0.9                         ── E: 0     | W: 0     | N: 0card 0.1.0                             ── E: 0     | W: 0     | N: 1bundle 0.1.0                           ── E: 1     | W: 0     | N: 0butcher 0.3.2                          ── E: 0     | W: 0     | N: 0caret 6.0.94                           ── E: 0     | W: 0     | N: 0correlationfunnel 0.2.0                ── E: 0     | W: 0     | N: 1CSCNet 0.1.2                           ── E: 0     | W: 0     | N: 0                                               
I D2MCS 1.0.1                            ── E: 1     | W: 0     | N: 0cvms 1.6.0                             ── E: 0     | W: 0     | N: 0easyalluvial 0.3.1                     ── E: 0     | W: 0     | N: 0DALEXtra 2.3.0                         ── E: 0     | W: 0     | N: 0embed 1.1.1                            ── E: 0     | W: 0     | N: 1finetune 1.1.0                         ── E: 0     | W: 0     | N: 0hardhat 1.3.0                          ── E: 0     | W: 0     | N: 0healthcareai 2.5.1                     ── E: 2     | W: 1     | N: 1finnts 0.2.4                           ── E: 0     | W: 0     | N: 0healthyR.ts 0.2.9                      ── E: 0     | W: 0     | N: 1HTRX 1.2.2                             ── E: 0     | W: 0     | N: 1                                               
I hydrorecipes 0.0.3                     ── E: 1     | W: 0     | N: 0healthyR.ai 0.0.13                     ── E: 0     | W: 0     | N: 0MLDataR 1.0.1                          ── E: 0     | W: 0     | N: 1modelgrid 1.1.1.0                      ── E: 0     | W: 1     | N: 2MachineShop 3.6.2                      ── E: 0     | W: 0     | N: 0modeltime 1.2.7                        ── E: 0     | W: 0     | N: 1modeltime.ensemble 1.0.3               ── E: 0     | W: 0     | N: 1palmerpenguins 0.1.1                   ── E: 0     | W: 0     | N: 0modeltime.resample 0.2.3               ── E: 0     | W: 0     | N: 1nestedmodels 1.0.4                     ── E: 0     | W: 0     | N: 0rules 1.0.2                            ── E: 0     | W: 0     | N: 0rsample 1.1.1                          ── E: 0     | W: 0     | N: 0shinyrecipes 0.1.0                     ── E: 0     | W: 0     | N: 1SomaDataIO 6.0.0                       ── E: 0     | W: 0     | N: 1probably 1.0.2                         ── E: 0     | W: 0     | N: 0sknifedatar 0.1.2                      ── E: 1     | W: 0     | N: 0sparseR 0.2.2                          ── E: 0  +1 | W: 0     | N: 0swag 0.1.0                             ── E: 0     | W: 0     | N: 0stacks 1.0.2                           ── E: 0     | W: 0     | N: 0tabnet 0.4.0                           ── E: 0     | W: 0     | N: 0stabiliser 1.0.6                       ── E: 0     | W: 0     | N: 1text 0.9.99.2                          ── E: 0     | W: 0     | N: 1textrecipes 1.0.3                      ── E: 0     | W: 0     | N: 1tfhub 0.8.1                            ── E: 1     | W: 0     | N: 0tidyAML 0.0.2                          ── E: 0     | W: 0     | N: 0tidybins 0.1.0                         ── E: 0     | W: 0     | N: 1themis 1.0.1                           ── E: 0     | W: 0     | N: 0tidyclust 0.1.2                        ── E: 0     | W: 0     | N: 0tidymodels 1.1.0                       ── E: 0     | W: 0     | N: 0tune 1.1.1                             ── E: 0     | W: 0     | N: 0usemodels 0.2.0                        ── E: 0     | W: 0     | N: 0vetiver 0.2.2                          ── E: 0     | W: 0     | N: 0timetk 2.8.3                           ── E: 0     | W: 0     | N: 1workboots 0.2.0                        ── E: 0     | W: 0     | N: 0waywiser 0.4.1                         ── E: 0     | W: 0     | N: 1workflows 1.1.3                        ── E: 0     | W: 0     | N: 0workflowsets 1.0.1                     ── E: 0     | W: 0     | N: 0                                               
OK: 62                                                                                                             
BROKEN: 1
Total time: 20 min

@EmilHvitfeldt EmilHvitfeldt merged commit f0aada7 into main Jul 18, 2023
9 checks passed
@EmilHvitfeldt EmilHvitfeldt deleted the missing_keep_original_cols branch July 18, 2023 18:45
@github-actions
Copy link

github-actions bot commented Aug 2, 2023

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 Aug 2, 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.

None yet

2 participants