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

copy in most unexported funs and hack environment #10

Merged
merged 1 commit into from
May 19, 2021

Conversation

brshallo
Copy link
Owner

Should fix #9 .

Copy in most exported functions from @carlomedina original fork carlomedina/dplyr. Most of these are in R/context.R

This on it's own did not fix things, also did hack that sets mask to dplyr (see pwiser:::context_peek_bare()). This is an absolute hack and means that checks on whether pairwise() is being used in appropriate contexts is no longer being checked...

Also updated README to add note about overlap with more mature package {dplyover} and limitations (pointing to #1).

@brshallo brshallo merged commit 3131adb into main May 19, 2021
@IndrajeetPatil
Copy link

Awesome, thanks!

Didn't know about dplyover, will check it out.

Are you planning to submit this package to CRAN at some point?

@brshallo
Copy link
Owner Author

Are you planning to submit this package to CRAN at some point?

Initially I was thinking after getting to issues # 1, 2 and (maybe) 8 might take steps to set-up with CRAN. (Note though that # 1 is a hurdle as current set-up is just modifying copied code from {dplyr} and neither Carlo nor I really have a firm understanding of how masking etc is working -- e.g. see hack indicated above 😅. So were debating rewriting removing dependencies on unexported funs and other things. (Shared publicly perhaps a bit prematurely but figured may as well see if people like idea of and get some feedback -- thanks for opening issue!)

After discovering {dplyover} am a bit less sure what will do with {pwiser} (for reasons now noted in README) and am starting talks with Tim (owner of package). My plan was in the next few weeks to do a deep dive on {dplyover}. If works the way I expect it to, I will likely just devote energy there and see if can help with some of the documentation and maybe giving feedback on interface etc. Tim is a lot further along and has a lot more comprehensive set of functionality (which I think has the possibility of becoming a relatively popular + useful {dplyr} extension.)

But will have a more firm idea after talking with Tim and reviewing dplyover a bit more.

@IndrajeetPatil
Copy link

Makes sense!

If there is an overlap of functionality and none of the packages are on CRAN, maybe it makes more sense to join forces and combine functionalities. :)

Either way, looking forward to using them in my workflow!

@brshallo
Copy link
Owner Author

Yeap, pwiser::pairwise() essentially represents a particular use of dplyover::across2(). Yes, Tim has plans of getting {dplyover} on CRAN, will likely devote energy there.

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.

example from README not working
2 participants