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

import from folder not working (at least on the mac) #12

Closed
manuelsantillan opened this issue Apr 30, 2014 · 13 comments
Closed

import from folder not working (at least on the mac) #12

manuelsantillan opened this issue Apr 30, 2014 · 13 comments

Comments

@manuelsantillan
Copy link

I was trying this module system, since I come from a node.js background and I think that R needs something like this really badly (congrats!)
However, I can only get to work the simplest case of same-folder modules. When I try to import modules in nested folders it doesn't work.

In my test scenario, I've got a config/log.r module I want to import. If I cd to config and import the module with import("log") it works. However, if I launch R from config's parent folder, it doesn't.

require(modules)
import("config/log")
Error en find_module("config/log") : 
  Unable to load module config/log; not found in "/Users/santillan/dev/solvview/analitix-r/src"

Seems that is_valid_nested considers my module is not valid. Is it mandatory to create a log folder and create a "init.r" file? Or should this just work?

@mschubert
Copy link
Collaborator

I've had the same discussion with @klmr yesterday, __init__.r files are mandatory for nested modules.

@klmr
Copy link
Owner

klmr commented Apr 30, 2014

it mandatory to create a log folder and create a "init.r" file?

In the example you’ve given, either of two things need to exist:

  1. A folder config, a file config/log.r and a file config/__init__.r (to mark config as being a module; or
  2. Folders config, and config/log, and files config/__init__.r and config/log/__init__.r.

So log may refer either directly to a file, or to a nested submodule which is itself a folder. But like @mschubert said, in either case you are required to have an __init__.r file in each nested module (precisely to mark it as a module, rather than just any folder).

@manuelsantillan
Copy link
Author

But Does it need to contain anything or it can be just a blank file?

El miércoles, 30 de abril de 2014, Konrad Rudolph notifications@github.com
escribió:

it mandatory to create a log folder and create a "init.r" file?

In the example you’ve given, either of two things need to exist:

  1. A folder config, a file config/log.r and a file config/init.r(to mark
    config as being a module; or
  2. Folders config, and config/log, and files config/init.r and
    config/log/init.r.

So log may refer either directly to a file, or to a nested submodule
which is itself a folder. But like @mschuberthttps://github.com/mschubertsaid, in either case you are required to have an
init.r file in each nested module (precisely to mark it as a module,
rather than just any folder).


Reply to this email directly or view it on GitHubhttps://github.com/klmr/modules/issues/12#issuecomment-41794860
.

Manuel Santillán
Socio - Director de Operaciones
www.solvver.com

@klmr
Copy link
Owner

klmr commented Apr 30, 2014

Ah, apparently I need to clarify (meaning, write) the documentation on this point: The file can be empty. It then just serves as a “marker” that the directory is a module. It can also contain code, which is executed when the module is loaded for the first time in a session (so doing import on the same module twice will only execute the code once).

At this point the behaviour is identical to that in Python, which is why I have forgotten to document it more clearly.

@manuelsantillan
Copy link
Author

Thnx!! I've got it working now, including intra-package references. It seems a bit overkill so many init.r files BTW. Have you considered a simpler approach, like node.js's module system? http://nodejs.org/api/modules.html

@manuelsantillan
Copy link
Author

Another, related, issue I'm finding is the following: if I have the following structure:
config/log.r
config/init.r
test/init.r
test/mytest.r

If I launch the test from the root folder

Rscript test/mytest.r 

then I've got to import:

log <- import("config/log")

But if I launch the test from the test folder then I've got to import:

log <- import("../config/log")

This means that imports are dependent on where is the code launched from, which seems to be a bad idea, don't you think so? Do you think this could be avoided? Does python do the same? In node.js, imports are only dependent on the relative path between the importing package and imported package, not on where the code is launched from...

@klmr
Copy link
Owner

klmr commented May 6, 2014

Actually, import('../config/log') should not even work, that’s a bug (which will be fixed in the future): .. is not supposed to be a valid module name part (module names are not paths, even if their hierarchy echoes the folder hierarchy).

The solution here is to configure your import.path R option so that the same path can be used in both cases. So for instance if you have some facility for running tests inside the test folder (let’s call it test-all.r, then you might have the following code in there:

option(import.path = '..')
import(mytest)
#

Of course that still hard-codes the knowledge that the tests will be run from within the test folder. Where that is not desired, you need to extract the path of the current file from the command line. R doesn’t make this particularly easy, unfortunately:

argv <- commandArgs(trailingOnly = FALSE)
fopt <- '--file='
scriptFilename <- sub(fopt, '', argv[grep(fopt, argv)])
dirname = basename(scriptFilename)

If you instead launch test-all.r as a module, it’s much easier: just use module_path().

@manuelsantillan
Copy link
Author

AFAIK, Python modules support relative references ('..' is not a path just a relative reference!). In python, this is called intra-package references and is a key necessity of a module system: https://docs.python.org/2/tutorial/modules.html see section 6.4.2

I really do think that a form of relative references is needed for any non-trivial use case for a module system. As a matter of fact, node.js' module system also uses dotted notation (as does python).

Think for example of a project that has modules for logging, shared functions among sub-modules, and so on.

@klmr
Copy link
Owner

klmr commented May 6, 2014

As far as I understand, intra-package references in Python are needed (only) because, for whatever reason, external modules take precedence over local modules. Say you have a module called foo installed inside /path/to/python/packages, and you also have a module ./foo.py inside your current package. Then import foo will import the global module rather than the local file, and you need from . import foo in order to overcome that.

My R modules go the other way: ./foo.r will always take precedence over an external module, analogous to conventional scoped name lookup rules. In order to access non-local modules, future versions of Modules will have import('foo', path = 'the/absolute/path') instead.

@manuelsantillan
Copy link
Author

Of course, it's your R modules, but it seems a bad idea to convolute importing a "brother" module with option(import.path...) or whatever, instead of sticking to one of the 2 mainstream approaches:

A. absolute path from the project root or via a URL-y namespace -> .NET, Java and the likes go this way. This would mean that If I have the following structure:
config/log.r
services/myservice.r
I could import, say, config/log.r from services/myservice.r with

import("config/log") 
#or if you prefer
import("/config/log")
#or even
import("myprojectname/config/log")

B. Accept relative paths -> python and node.js go this way.

import("../config/log") 

The key point is that it is not an edge case, but one of the most common use cases when using a module system.

@klmr
Copy link
Owner

klmr commented May 6, 2014

I agree with you in principle, I would simply have grouped Python with A instead of B, nothing that the relative import paths were welded on after the fact. The idea was to emulate Python’s system as far as it makes sense, but not inherit its mistakes.

But you’ve actually convinced me on this point. Python’s way of doing it does make sense.

@klmr klmr added this to the First final API milestone May 6, 2014
@manuelsantillan
Copy link
Author

Glad to hear that! Please let me know if I can help

cheers

@klmr
Copy link
Owner

klmr commented Jun 21, 2014

I think this is now implemented. Unfortunately it’s hard to test this comprehensively, since R segfaults when trying to invoke the relevant test cases (please refer to the code for details). Note that this is not a problem of modules … rather, it seems to be somehow caused by cross-talk between testthat and devtools.

I’d therefore appreciate if you could test the current preliminary version (I haven’t created a new release for now) on your code.

@klmr klmr closed this as completed Jun 26, 2014
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

3 participants