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

Proper use of restore and proxy #322

Closed
lionel- opened this issue May 19, 2021 · 3 comments · Fixed by #323
Closed

Proper use of restore and proxy #322

lionel- opened this issue May 19, 2021 · 3 comments · Fixed by #323

Comments

@lionel-
Copy link
Member

lionel- commented May 19, 2021

The vignette showcases improper use of restore and proxy:

var_ <- function(x, ...) {
  out <- var(x, ...)
  vctrs::vec_restore(out, vctrs::vec_proxy(x))
}

Here var_ is restoring to the proxy. The correct procedure is: Take the proxy, operate on it (e.g. with var()), restore to input.

I think it'd be a good idea to mention this is a rather experimental use of proxy and restore. Actually I'd prefer not showing this pattern until we've thought through numerical operations.

@krlmlr
Copy link
Member

krlmlr commented May 21, 2021

Should the correct use of vec_proxy() and vec_restore() be documented in ?vec_proxy ?

@lionel-
Copy link
Member Author

lionel- commented May 21, 2021

There is currently no documentation for creating new primitive vctrs operations since all these patterns are currently internal. Documenting this is on the long term horizon. Currently, users are expected to use the existing operations like vec_slice().

krlmlr added a commit that referenced this issue May 21, 2021
- Fix documentation on usage of `vctrs::vec_proxy()` and `vctrs::vec_restore()` (#322).
@github-actions
Copy link
Contributor

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue and link to this old issue if necessary.

@github-actions github-actions bot locked and limited conversation to collaborators May 22, 2022
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 a pull request may close this issue.

2 participants