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

Allow tidyselect-like syntax within use() #319

Open
wurli opened this issue May 4, 2023 · 2 comments
Open

Allow tidyselect-like syntax within use() #319

wurli opened this issue May 4, 2023 · 2 comments

Comments

@wurli
Copy link

wurli commented May 4, 2023

This could be seen as an extension of #287.

E.g. use(purrr[contains("map")]) would import map(), imap(), map_chr(), map_dbl() etc. Another common pattern might be use(ggplot2[ggplot, aes, starts_with("geom_")]). In my opinion this would do a lot for readability/convenience, and would lend familiarity for those who already use the tidyverse.

The only possible issue I can see is that tidyselect would use everything() rather than ... to import the whole namespace. I suppose supporting both might be possible, but perhaps not very neat.

@klmr
Copy link
Owner

klmr commented Jun 4, 2023

Do you have a more concrete use-case in mind where this would be worthwhile? At the moment I am having a hard time justifying the complexity this would add (both to the code base and to the documentation). In particular, the examples you listed can already be expressed by either importing all desired names explicitly (recommended) or by using the wildcard import ... (generally not recommended). And while I can kind of see the “convenience” argument, I am not convinced that this improves readability.

Fundamentally, using these functions has all the disadvantages of a wildcard import, because it attaches implicit names. The main issue is that this can silently break user code when a module adds new exports that now override other imports.

I’m also comparing with module systems in other languages, and as far as I can see none of them offers an equivalent of what you’re proposing — indicating that this isn’t an important feature.

@wurli
Copy link
Author

wurli commented Jun 16, 2023

I think your arguments against this are solid, so if you decide it's not worth adding, fair enough. I think my main reason for the suggestion is this is that I don't want to have to reuse lengthy use() statements at the tops of all my modules. If I want all map() functions I'd rather use(purrr[contains("map")]) then have to manually include all 62 variations. I agree it's less robust, but I think it's clearer in some sense - e.g. it's not easy to confirm you haven't forgotten any map() variations with the current system.

Having said that, the more I think about it, the more I think this feature is at odds with the core purpose of {box}.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants