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

Restructuring API #199

Closed
tkf opened this issue Sep 6, 2018 · 5 comments
Closed

Restructuring API #199

tkf opened this issue Sep 6, 2018 · 5 comments

Comments

@tkf
Copy link
Member

tkf commented Sep 6, 2018

Here are some ideas on refactoring PyJulia APIs and internals. Mainly, I suggest to introduce two new classes JuliaInfo and LibJulia.

JuliaInfo --- libjulia discovery mechanism

I suggest to turn JuliaInfo into a proper class. It is at the moment a namedtuple:

pyjulia/julia/core.py

Lines 263 to 267 in cdbefb1

JuliaInfo = namedtuple(
'JuliaInfo',
['JULIA_HOME', 'libjulia_path', 'image_file',
# Variables in PyCall/deps/deps.jl:
'pyprogramname', 'libpython'])

For example, we have is_compatible_python which is better be organized as a method.

class JuliaInfo:

    @classmethod
    def load(cls, executable="julia"):
        """
        Get basic information from Julia `executable`.
        """

    executable = ...
    """ Julia executable from which information is retrieved. """

    bindir = ...
    """ Sys.BINDIR of `.executable`. """

    libjulia_path = ...
    """ Path to libjulia. """

    python = ...
    """ Python executable with which PyCall.jl is configured. """

    libpython_path = ...
    """ libpython path used by PyCall.jl. """

    def is_compatible_python(self):
        """
        Check if python used by PyCall.jl is compatible with `sys.executable`.
        """

LibJulia --- Low-level C API

Julia has a rather lengthy __init__ which mainly setting up another object Julia.api. This is probably better be organized into another class.

Importantly, I suggest to make LibJulia.init_julia lazy (by default) and idempotent. It lets users to run functions that cannot be called after the initialization (e.g., jl_parse_opts; ok I'm not sure if there are something else) while integrating with high-level API provided by PyJulia.

def setup_libjulia(libjulia):
    libjulia.jl_eval_string.argtypes = [c_char_p]
    libjulia.jl_eval_string.restype = c_void_p
    # and so on...


class LibJulia:

    """
    Low-level interface to `libjulia` C-API.
    """

    def __init__(self, juliainfo, init_julia=None):
        libjulia_path = juliainfo.libjulia_path
        libjulia = ctypes.PyDLL(libjulia_path, ctypes.RTLD_GLOBAL)
        self.libjulia = libjulia
        self.juliainfo = juliainfo
        setup_libjulia(libjulia)

        if init_julia is not None:
            if init_julia:
                self.init_julia()
            else:
                # If `init_julia = False` is passed, remember that
                # libjulia should never be initialized again for this
                # Python process.
                self.__class__._julia_initialized = True

    _julia_initialized = False

    def init_julia(self):
        """
        Initialize `libjulia`.  Calling this method twice is a no-op.

        It calls `jl_init_with_image` (or `jl_init_with_image__threading`)
        but makes sure that it is called only once for each process.
        """
        if self._julia_initialized:
            return
        jl_init_path = self.juliainfo.bindir
        image_file = self.juliainfo.image_file.encode("utf-8")
        self.libjulia.jl_init_with_image(jl_init_path, image_file)
        # or maybe just jl_init()?

        self.__class__._julia_initialized = True
        # Using `self.__class__._julia_initialized` rather than
        # `self._julia_initialized` to have a unique flag within a
        # process.

I added some jl_unbox_* functions in another PR #186 (a954914). Adding those C API could be useful, too.

Julia --- High-level PyCall-based API

Since low-lvel libjulia handling is decoupled, Julia class now can focus on high-level API based on PyCall. I also suggest to move some "free functions" (e.g., isdefined) whose first argument was julia to methods.

class Julia:  # or maybe JuilaAPI
    """
    High-level Julia API.
    """

    capi = ...
    """ An instance of `.LibJulia` """

    def __init__(self, capi):
        self.capi = capi
        self.capi.init_julia()

    # Some functions are better be defined as methods:

    def isdefined(self, parent, member):
        ...

    def isamodule(self, julia_name):
        ...

    def isafunction(self, julia_name, mod_name=""):
        ...

get_julia

Since initializing Julia API now takes a few steps, I suggest to add a factory function which is equivalent to current julia.core.Julia.

def get_julia(init_julia=True, runtime=None, bindir=None):
    """
    Create a Python object that represents a live Julia interpreter.
    """
    return Julia(LibJulia(JuliaInfo.load(runtime), init_julia))
@stevengj
Copy link
Member

Rather than a get_julia function, why not just have a Julia() default constructor?

@tkf
Copy link
Member Author

tkf commented Sep 10, 2018

Since __init__ is not a normal method (e.g., you cannot return from it), I found that supporting multiple behaviors of __init__ is not an idiomatic Python. In Python, I think it's more natural to have different class methods or factory functions to support different ways to construct the object. I do like Julia's approach but I think it's better to take a natural approach for Python here. For example, if you support multiple call signatures of __init__ then it's hard to inherit the class (I don't say it's a good usage to inherit Julia class, but if it's hard to do it then I think there is something bad in the design).

Python standard libraries use factory functions as API too, e.g.: https://docs.python.org/3/library/ipaddress.html

Having said that, it's not super difficult to support multiple signatures of __init__ while behaving nicely; by that, I mean it has to raise TypeError when the combination of the arguments is wrong (e.g., Julia(libjulia, runtime="some_julia") must raise TypeError because runtime would not be used) and also easy to extend (though I'm not promoting inheritance over composition):

class Julia:

    def __init__(self, libjulia=None, **kwargs):
        if libjulia is None:
            libjulia, kwargs = self.default_libjulia(**kwargs)
        self.setup(libjulia, **kwargs)

    @staticmethod
    def default_libjulia(runtime=None, bindir=None, **unused):
        return (LibJulia(JuliaInfo.load(runtime=runtime, bindir=bindir)),
                unused)

    def setup(self, libjulia, init_julia=True):
        self.libjulia = libjulia
        if init_julia:
            libjulia.init_julia()

@stevengj
Copy link
Member

stevengj commented Sep 10, 2018

My thinking is that 99% of pyjulia users will want the default options, so it seems nicer to just support this with Julia() in order to minimize the number of functions needed in the API, checking the __init__ arguments as needed for other possibilities.

@tkf
Copy link
Member Author

tkf commented Sep 10, 2018

My guess is that most Julia users would just use from julia import Main for Main.eval or just directly from julia import SomeModule. I said that Julia class is a high-level API but it's a lower level compared to those "direct import" highest-level API.

I'm still slightly uneasy to do all the initializations in Julia.__init__. Julia API is a process-level "singleton" resource so I want an API that gives users the right impression just from the class/function names.

For Julia users, most of the reason why they call the custom initialization routine (get_julia) would be because they want to switch Julia version (and sys image?) and not because they want API methods provided by Julia. In that case, they just need to know one more function to setup Julia API, not two (i.e., + constructor). Thinking along this line, maybe it should be called setup instead of get_julia. It would look like:

import julia
julia.setup("julia-0.7")  # a Julia instance would be returned but it's OK to ignore
from julia import Main
Main.eval('@assert VERSION < v"1"')

If they want to use Julia class API then it means they want more Python methods. Why not let them know there are higher and lower level initialization functions they can call.

@tkf
Copy link
Member Author

tkf commented Mar 23, 2019

This was implemented in #235. See: https://pyjulia.readthedocs.io/en/latest/api.html

It turned out I didn't need to touch Julia API at all. This is because the Julia runtime can be initialized only once. So, there was not much communication needed between Julia and LibJulia as I was anticipated.

@tkf tkf closed this as completed Mar 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants