Make module loading lazy #162

tkf · 2018-05-15T20:24:34Z

@stevengj suggested to make JuliaModule lazy: #159 (comment)

Here is my implementation for that. I actually added some more changes on top that to solve relevant issues. If some components of the changes have to be removed, let me know. Those components are implemented roughly in the following order:

Basing on CI fix #149

~~First of all this PR is based on #149 to run CI. Let me know if I need to rebase once #149 is merged.~~

(Edited: now rebased)

Lazy module member loading

Julia.eval is called via JuliaModule.__getattr__ so that Julia object is brought to Python world only when it is requested.

Implemented in dc40166 and finished by 843b519

Julia's interpreter global namespace as a module `julia.Main`

In addition to usual JuliaModule, the module class JuliaMainModule of julia.Main has an implementation of __setattr__ so that you can send Python object to Julia by just setting it:

>>> from julia import Main
>>> Main.xs = [1, 2, 3]

Since it supersede core.Julia, I suggest to deprecate accessing Julia values via core.Julia().<name>: b30af45

Automatically initialize Julia with `from julia import ...`

This makes from julia import <Julia module> works without the initial setup. Note that a custom setup can still be done by calling julia.Julia with appropriate arguments before trying to import Julia modules:
51997c0

See also the document on the new API:
https://github.com/tkf/pyjulia/blob/lazy-module/README.md#usage

tkf · 2018-05-15T20:30:11Z

julia/core.py

+                split_path = module_path.split(".")
+                is_base = split_path[-1] == "Base"
+                recur_module = split_path[-1] == split_path[-2]
+                if not is_base and not recur_module:


Not sure what is the best way to handle submodules. Is there an easy way to check if some module is a submodule of other module in Julia? If not, it's probably better to let user explicitly import submodules (i.e., julia.Module1.Module2 is an AttributeError until the user does import julia.Module1.Module2)?

tkf · 2018-05-15T23:40:29Z

In the current master, we have "jl".join(name) in JuliaModuleLoader when Julia name is a Python keyword:

pyjulia/julia/core.py

Lines 77 to 88 in 7ebad22

    
           names = self.julia.eval("names({}, true, false)".format(juliapath)) 
        
           for name in names: 
        
               if (ismacro(name) or 
        
                   isoperator(name) or 
        
                   isprotected(name) or 
        
                   notascii(name)): 
        
                   continue 
        
               attrname = name 
        
               if name.endswith("!"): 
        
                   attrname = name.replace("!", "_b") 
        
               if keyword.iskeyword(name): 
        
                   attrname = "jl".join(name)

What was the intention of this code? I mean, "jl".join("with") evaluates to 'wjlijltjlh' which is not a very human-friendly name.

Was it probably not by intention? Was it maybe intended to be "jl" + name or something?

At the moment, this PR has no code for Python keyword handling. We can add something like:

if name.startswith("jl_"):
    try:
        return self.__try_getattr(name[len("jl_"):])
    except AttributeError:
        pass

tkf · 2018-05-16T02:48:15Z

test/test_core.py

+    def test_import_without_setup(self):
+        command = [sys.executable, '-c', 'from julia import Base']
+        print('Executing:', *command)
+        subprocess.check_call(command, env=_orig_env)


env=_orig_env probably is required to fix https://travis-ci.org/JuliaPy/pyjulia/jobs/379514563#L298

Yes it looks like it fixes the failure: https://travis-ci.org/JuliaPy/pyjulia/builds/379524930

rdeits · 2018-06-20T07:04:08Z

CI is now fixed, so you might want to rebase this PR

tkf · 2018-06-20T21:01:53Z

Thanks. Rebased.

stevengj · 2018-06-21T15:38:11Z

julia/core.py

+    def __try_getattr(self, name):
+        juliapath = remove_prefix(self.__name__, "julia.")
+        try:
+            module_path = ".".join((juliapath, name))


maybe qualified_name or something? Calling this a "path" is a bit confusing.

stevengj · 2018-06-21T15:39:50Z

julia/core.py

+                    return self.__loader__.load_module(newpath)
+            return self._julia.eval(module_path)
+        except JuliaError:
+            if isafunction(self._julia, name, mod_name=juliapath):


I'm confused by this isafunction check. Didn't it already try self._julia.eval(module_path) for all objects that are not modules, including functions? And if that threw an error, shouldn't we rethrow it?

stevengj · 2018-06-21T15:41:45Z

julia/core.py

+
+    def __try_getattr(self, name):
+        juliapath = remove_prefix(self.__name__, "julia.")
+        try:


instead of this try block, maybe we should try calling isdefined(module, :name), throwing an AttributeError if it returns false, and letting any JuliaError that is thrown fall through to the caller.

tkf · 2018-06-22T00:21:49Z

@stevengj Thanks for the code review!

I renamed module_path to jl_fullname reflecting that we are calling it fullname in Python side: https://docs.python.org/3/library/importlib.html#importlib.abc.Loader.load_module (Make module loading lazy #162 (comment))
Yes, isafunction-then-julia.eval does seem redundant. I must have copied the existing code without careful reconsideration. It's removed now. (Make module loading lazy #162 (comment))
try block is replaced with isdefined as you suggested (Make module loading lazy #162 (comment))

Some additional changes:

I moved DeprecationWarning and compatibility layer to julia.core.LegacyJulia and expose it as julia.Julia. It is not strictly necessary but it was fragile to have julia.Julia.__getattr__ forwarded to juila.Main.__getattr__ as it created infinite recursion (resulting in segfaults) when I introduced some bug while editing the code. Alternatively, I can separate the PR for the new API if you want to focus on reviewing the lazy loader.
I removed the import from isamodule. Without this change, trying to access undefined variables such as julia.Base.__path__ produces a warning from Julia before JuliaModule.__getattr__ raising AttributeError, since now isamodule is called via JuliaModule.__getattr__.

tkf · 2018-06-22T01:47:34Z

julia/core.py

@@ -140,8 +140,6 @@ class JuliaImporter(object):

    # find_module was deprecated in v3.4
    def find_module(self, fullname, path=None):
-        if path is None:


It looks like this code was added in fa157b1 and carried around as-is.

Probably just a remnant of experimental code?

kmsquire

I made a few suggestions regarding wording changes.

I like the changes, especially being able to import Julia modules without setup.

However, I find that when I run ipython3 from the command line, I get a segfault during tab completion of anything in a Julia module:

# pip3 install ipython # if necessary
> ipython
Python 3.6.5 (default, Jun 17 2018, 12:13:06)
Type 'copyright', 'credits' or 'license' for more information
IPython 6.4.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: import julia

In [2]: from julia import Base

In [3]: Base.<tab>[1]    79420 segmentation fault  ipython

master does not have this problem. Using regular python3 (instead of ipython3) does not have this problem.

kmsquire · 2018-07-26T11:44:59Z

README.md

+Main.xs = [1, 2, 3]
+```
+
+so that, e.g., it can be evaluated at Julia side using Julia syntax:


Perhaps change the wording to:
which allows it to be accessed directly from Julia code, e.g.,

(While one can do this, I think it's better to pass values via function call arguments, rather than rely on values existing in global scope.)

I totally agree that communication-by-sharing is a terrible idea and using this feature in Python modules should be forbidden :) But I was thinking in terms of interactive usage, mostly via %julia. In that case, passing around code via global namespace is pretty useful.

kmsquire · 2018-07-26T11:48:29Z

README.md

+### Low-level interface
+
+If you need a custom setup for `pyjulia`, it must be done *before* any
+imports of Julia modules.  For example, to use Julia interpreter at


Suggestion for some (minor) wording changes:

If you need a custom setup for `pyjulia`, it must be done *before* importing any Julia modules. For example, to use the Julia interpreter at

kmsquire · 2018-07-26T11:58:52Z

README.md

+### High-level interface
+
+To call a Julia function in Julia module, import the Julia module (say
+`Base`) by:


Suggestion for (minor) wording changes:

To call a Julia function in a Julia module, import the Julia module (say `Base`) with

tkf · 2018-07-31T16:45:11Z

Hi @kmsquire, thanks a lot for correcting my English!

BTW, it's interesting that you find that IPython tab completion works in pyjulia master. I thought, in theory, pyjulia would never work with IPython < 7.0 since the tab-completion is done in different thread #132 (comment) and pyjulia (or rather libjulia, I guess) is not thread-safe. I actually find out that IPython master (= 7.x) already works in single-thread just now. I have to check it...

tkf · 2018-07-31T18:26:40Z

Test failure is due to CI setting. See #171.

stevengj · 2018-07-31T20:13:18Z

Is this ready to merge? I haven't really been keeping up with pyjulia…

tkf · 2018-07-31T20:57:52Z

I think so. I mean, I used this branch for a few weeks just after creating it and I don't remember encountering any regressions. After that, the only change was in README. I'm not planning to add anything more to this PR.

It adds an easier way to set variables in Julia's global namespace: >>> from julia import Main >>> Main.xs = [1, 2, 3]

fixes JuliaPy#144

This makes `from julia import <Julia module>` works without the initial setup. Note that a custom setup can still be done by calling `julia.Julia` with appropriate arguments *before* trying to import Julia modules. closes JuliaPy#39, JuliaPy#79

Otherwise, trying to access undefined variables such as `julia.Base.__path__` produces a warning from Julia before `JuliaModule.__getattr__` raising `AttributeError`.

as Base.REPL was pulled out from Base in Julia 0.7.

tkf · 2018-07-31T22:48:53Z

Rebased onto master. Travis and AppVeyor are all green now.

tkf commented May 15, 2018

View reviewed changes

tkf force-pushed the lazy-module branch from 4220ba1 to 0493e3c Compare May 15, 2018 23:28

tkf force-pushed the lazy-module branch 4 times, most recently from d8c6bb8 to d057d62 Compare May 16, 2018 02:03

tkf commented May 16, 2018

View reviewed changes

tkf force-pushed the lazy-module branch from 5323801 to 5053864 Compare May 16, 2018 02:54

tkf force-pushed the lazy-module branch from 5053864 to 8b45e80 Compare June 20, 2018 20:59

stevengj reviewed Jun 21, 2018

View reviewed changes

tkf force-pushed the lazy-module branch from 1b1e859 to 3564677 Compare June 22, 2018 00:37

tkf commented Jun 22, 2018

View reviewed changes

kmsquire reviewed Jul 26, 2018

View reviewed changes

tkf added 7 commits July 31, 2018 15:08

Make module loading lazy

8e620a2

Catch less in JuliaModule.__getattr__

a9c00fb

Support star import

b878547

Special handling for Main module

1c4a268

It adds an easier way to set variables in Julia's global namespace: >>> from julia import Main >>> Main.xs = [1, 2, 3]

Deprecate Julia.__getattr__ (and make it lazy)

d7c3fb0

fixes JuliaPy#144

More interfaces to JuliaMainModule

6ab2f79

tkf added 12 commits July 31, 2018 15:08

Document the new API

c586e42

Properly remove prefix

f306f79

Fix JuliaModule.__all__; add .__dir__

25ee1c4

Cache original os.environ for tests

6a74217

Add LegacyJulia to break infinite recursion

994ab48

Simplify JuliaModule.__try_getattr

096a3c9

Don't import in isamodule

208c3b6

Otherwise, trying to access undefined variables such as `julia.Base.__path__` produces a warning from Julia before `JuliaModule.__getattr__` raising `AttributeError`.

Remove core._orig_env; it can be done in test code

86d9ab1

Use Enums as example sub-module

7849839

as Base.REPL was pulled out from Base in Julia 0.7.

More import tests

28ce894

Remove noop code

15ec596

Fix wording

ea4492f

tkf force-pushed the lazy-module branch from d5f69ae to ea4492f Compare July 31, 2018 22:08

stevengj merged commit e617ad9 into JuliaPy:master Aug 1, 2018

This was referenced Aug 1, 2018

add documentation to use julia module #79

Closed

A script for reproducing the initialization error #175

Closed

Avoid recursion during error handling #176

Merged

Make module loading lazy #162

Make module loading lazy #162

Uh oh!

Conversation

tkf commented May 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Basing on CI fix #149

Lazy module member loading

Julia's interpreter global namespace as a module julia.Main

Automatically initialize Julia with from julia import ...

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tkf commented May 15, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rdeits commented Jun 20, 2018

Uh oh!

tkf commented Jun 20, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tkf commented Jun 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kmsquire left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tkf commented Jul 31, 2018

Uh oh!

tkf commented Jul 31, 2018

Uh oh!

stevengj commented Jul 31, 2018

Uh oh!

tkf commented Jul 31, 2018

Uh oh!

tkf commented Jul 31, 2018

Uh oh!

Uh oh!

tkf commented May 15, 2018 •

edited

Loading

Julia's interpreter global namespace as a module `julia.Main`

Automatically initialize Julia with `from julia import ...`