search

rticulate / import

An Import Mechanism For R
https://import.rticulate.org
Other
222 stars 14 forks source link

Documentation doesn't match behavior for current github version #28

Closed mmuurr closed 3 years ago

mmuurr commented 4 years ago

In the "modules" section of the documentation, the behavior doesn't match the description. I explicitly copied the some_module.R example (and swapped out the library(ggplot2) line with the suggested import::here(qplot, .from = ggplot2) line, then tried to import::from that module like so:

 > import::from(some_module.R, a, b, p, plot_it)
 Error in qplot(Sepal.Length, Sepal.Width, data = iris, color = Species) :
   could not find function "qplot"

I'm using the most recent commit on the master branch (8ad48cc7e54bab65f2fcb816349dd221a73a1f20).

mmuurr commented 4 years ago

I walked back through commits and found the this is the commit that broke the module functionality (01c27bf3c7ffda2f3e81656fe5d4ca1d5e109d34), if that helps track down the issue.

mmuurr commented 4 years ago

@smbache I cloned the repo and then noticed you already have a branch that addresses this issue. I installed that branch and it seems to solve the problem ... should that be merged into master with a version bump for R package managers to detect?

Cheers!

torfason commented 4 years ago

Just a quick heads up. I've merged the fix-import-here-environment branch into the work I am doing, since I came across similar issues as well and found that it solved it. So once my pull request (https://github.com/smbache/import/pull/26) gets incorporated, that fix should get into the master branch.

smbache commented 4 years ago

Thanks! That's great

torfason commented 4 years ago

It feels to me that this issue (import::here() not working correctly) has now been fixed on master. Testing the most recent version would be appreciated: remotes::install_github("smbache/import")

Any remaining issues are probably caused by the recursive use of import (using import::from() in a module, that is then itself imported using import::from()). That issue can be fixed by using import::here() in the module file insted of import::from()(as @mmuurr was trying to do).

Any further discussions about the recursive import issue should probably be directed to #13 for now.

torfason commented 3 years ago

Fixed in release 1.2.0