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

Option to import all functions from a script without @export decorator #207

Closed
GitHunter0 opened this issue May 14, 2021 · 3 comments · Fixed by #227
Closed

Option to import all functions from a script without @export decorator #207

GitHunter0 opened this issue May 14, 2021 · 3 comments · Fixed by #227

Comments

@GitHunter0
Copy link

Hey @klmr , this package is really fantastic, I'm starting to use it everywhere.

I'd like to know if it is possible to import all functions from a script without the need to decorate each one with #' @export. That would be very helpful because it would allow us to import any existing script as a module.

Thank you

@klmr
Copy link
Owner

klmr commented May 15, 2021

I’d love to implement this (and seriously thought about how before the 1.0 release) but I just can’t think of a way to do this without major issues. If somebody can find a way, or can convince me that the options below are feasible, I’m happy to implement this.

  1. Have a separate function besides box::use for “legacy modules” (i.e. scripts). This totally defeats the purpose of having a single user-facing function.
  2. Add syntax for importing all names, similar to what we currently do with the attach specification box::use(my/mod[...]), but even non-exported ones — OK, but how?
  3. Add a function that could be called inside a module to manually define that everything should be exported (e.g. box::export_all()). This is feasible and might independently appear in a future version of ‘box’ as an alternative way of declaring exports. But it defeats the purpose, since now the raw script needs to be modified after all.
  4. Similar to (1), but add a keyword argument to box::use — .legacy = TRUE? IMHO even worse than (1).
  5. Similar to (2)/(4), but use some kind of prefix before the use declaration instead of a keyword argument. I.e. something like box::use(legacy:my/mod). Ideally the syntax would harmonise with whatever Auto-use (optionally) R's 'standard library'? #200 would use.

Even if the above could be solved there’d be another issue since scripts might be doing things that modules mustn’t do, such as calling library or source (see also #206). We’d have to intercept these calls in legacy modules and make them work within a module context (i.e. no global attach).

@GitHunter0
Copy link
Author

Thanks @klmr , it would be indeed very cool to have this feature available in box, but I see your points and for sure it is a challenge.

(1) I actually see no big problem in it, I kind of like it to have a separate function only for scripts ('legacy modules'), it makes clearer to the user what it is being imported and how.

(2) I don't know either...

(3) I agree. It defeats the purpose but can be useful, at least better than having to add all the decorators one by one.

(4)/(5) To keep box::use() universal, why not use the file extension as a cue to activate the 'legacy' mode? For example: box::use("my_script.R"), since it ends with .R, box would know it is an R script which requires the legacy importation mode. Allowing the users to specify a complete path string would also be nice, for example: box::use("C:/folder/script.R").

(6) Thinking about the future, a killer feature would be to use the same "file extension cue method" to import Python scripts via reticulate and .Rmd notebooks. I know it is a very complex task but we can't avoid having big aspirations, right?

Even if the above could be solved there’d be another issue since scripts might be doing things that modules mustn’t do, such as calling library or source (see also #206). We’d have to intercept these calls in legacy modules and make them work within a module context (i.e. no global attach).

Other difficult task, however If you could select the importation of only user-defined functions from the script (which is usually the main interest), most of that issues would go away, wouldn't? Since following good practices they should not contain library() or source()

@klmr
Copy link
Owner

klmr commented Jul 14, 2021

Another alternative that was originally considered but rejected, but to which I’m starting to come around:

If a module has no exports, consider it a legacy module. This means that all visible names will be exported (i.e. exclude only names starting with .). Legacy modules will probably also require other changes in semantics.1
To create a non-legacy module without exports, a module file now needs to be explicitly declared as a box module. This could either happen via a Roxygen-like comment (#' @box_module, or #' @module? — though the latter is courting potential compatibility issues with future versions of Roxygen) or via some kind of declaration in code, such as box::module().

Unfortunately this would be a breaking change. But I don’t judge the impact as severe, so I’m not averse to introducing this change in a minor release.


1 In particular, intercepting library, require and source in such a way as to emulate their semantics using box::use, rather than issuing a warning.

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

Successfully merging a pull request may close this issue.

2 participants