-
-
Notifications
You must be signed in to change notification settings - Fork 48
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
Consider making ‘box’ namespace environments actual namespaces #341
Comments
This may break a PR we have for |
Ah, thanks for letting me know. I think it should still work, but at any rate now I know about it and will pay attention to not breaking it. Also, great work on the ‘covr’ PR! |
Thanks! If you do change to namespace environments, I'll also take a look in the The |
As someone who's tried this before in a personal R package, this is a bad idea. Namespaces are expected to have a consistent structure that You would also run into the issue that a namespace is expected to originate from a directory with sub-directories help, Meta, R, possibly others such as html and libs, as well as files DESCRIPTION, INDEX, MD5, and NAMESPACE. box modules do not offer such a file structure, so they should not pretend to be package namespaces. Then comes the issue of namespace registration. Namespaces need to be registered, and you can register them, technically; here is some code at the C level:
and at the R level:
but this is a horrible idea. Even if you can justify registering your box namespaces in On the topic of serialization, namespaces are serialized in a manner unlike normal environments. See these links:
This means that no matter which name you decided to give to the box namespaces, they will always be serialized and unserialized as real namespaces. When being restored, the box namespaces will become real namespaces that link to the wrong environment, or will become There is another issue in that box namespaces can import variables from a package and use them. However, given that these are not actual namespaces, they are not registered as using those namespaces, meaning a real namespace can be unloaded even though it is in use by a box namespace. This might not break any R code, but all pointers to C or Fortran functions in that package will be set to As someone who has wasted tons of time trying to do the same thing myself, please don't waste your own time trying this. |
Thanks for this detailed explanation! Namespace registration and serialisation are definitely good points which all but forbid doing this. — The naming ambiguity could be handled, but that would defeat the purpose of having a “nice” printable namespace name. In other words, there doesn’t seem to be a point to making ‘box’ module namespaces into package namespaces. |
See discussion at klmr#341.
‘box’ module “namespace” environments are currently not actually namespaces as far as R is concerned (i.e. they don’t have a
.__NAMESPACE__.
environment with a nestedspec
name of type string).We should explore what consequences changing this would have. See:
box/R/env.r
Lines 21 to 22 in 2cef22e
On the positive side, it would lead to a nicer printing of the closure environment of a function defined inside a module. On the flip side, there might be rather a lot of unintended, subtle side-effects that current tests don’t capture, and which could break user code.
The text was updated successfully, but these errors were encountered: