Skip to content

store y_var in fit output from xy interface when available #961

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

Merged
merged 3 commits into from
Jun 8, 2023
Merged

Conversation

simonpcouch
Copy link
Contributor

Related to tidymodels/workflows#131. That issue notes that augment.workflow() doesn't return .resid as does the equivalent model fit from augment.model_fit(), even though it calls the model_fit methods in its internals. This is because workflows always(?) transitions to the xy interface before handing data off to parsnip. I came across this while putting together the companion PR to #960.

While the xy machinery makes use of the ..y placeholder in its internals, we can append the actual names attached to y—that are, at least in the workflows case, still intact—as y_var to the resulting model_fit so that we can reference that column later.

In combination with a workflows PR that will follow up on this one, augment.workflow() will return residuals in the situations that augment.model_fit() does!

Copy link
Member

@hfrick hfrick left a comment

Choose a reason for hiding this comment

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

I looked at this more from a "pure parsnip" perspective, to complement your workflow lense and found two use cases that you are unlikely/not going to run into when fitting through workflows but should get some attention anyways. Other than that: 👍 😄

And just as an aside:
re workflows "always(?)" transitioning to the xy interface: IIRC, it goes through fit() if the workflow has a "model formula" aka something in the formula arg to add_model(). That's used when you need to specify specific terms or use specific syntax that say a recipe can't understand. Examples are strata() terms for survival analysis or random effects in hierarchical models.

@simonpcouch simonpcouch requested a review from hfrick May 15, 2023 17:54
@simonpcouch
Copy link
Contributor Author

## revdepcheck results

We checked 56 reverse dependencies, comparing R CMD check results across CRAN and dev versions of this package.

 * We saw 0 new problems
 * We failed to check 0 packages

🚀

@simonpcouch simonpcouch requested a review from topepo May 15, 2023 20:35
@simonpcouch
Copy link
Contributor Author

Deferring to you on merge of this PR, @topepo!

@topepo topepo merged commit d104fa1 into main Jun 8, 2023
@topepo topepo deleted the y_var-xy branch June 8, 2023 13:09
@github-actions
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 Jun 23, 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.

3 participants