Why is there no explicit sys module in lib/python

[bernhard]

Heya!

I find it a little bit irritating that there's no explicit omero.sys module and in particular no omero.sys.ParametersI class in the python API. I have to write
from omero_sys_ParametersI import ParametersI

Can't you add the same mechanism for the sys module as for the model module?

By the way, if it's not too long to explain: why do you have the omero_*_* files anyway, is this how slice2py spits out the python API?
Where is the content of the individual modules defined, I partly can understand the omero.api definition from the corresponding __init__.py file, pointing to omero_API_ice.py but how's the same done for omero.model?

[jmoore]

I find it a little bit irritating that there's no explicit omero.sys module and in particular no omero.sys.ParametersI class in the python API. I have to write
from omero_sys_ParametersI import ParametersI

Can't you add the same mechanism for the sys module as for the model module?

Sure. Wanna submit a patch?

By the way, if it's not too long to explain: why do you have the omero_*_* files anyway, is this how slice2py spits out the python API?

Yes, the underscores are in almost all cases a byproduct of slice2py. A few files (like omero_sys_ParametersI and omero_model_{Details,Permissions}I) we've kept with the same naming, though it's not strictly necessary.

Where is the content of the individual modules defined, I partly can understand the omero.api definition from the corresponding __init__.py file, pointing to omero_API_ice.py but how's the same done for omero.model?

That all the model objects are loaded into omero.model is a by-product of the ObjectFactory doing it's work, not actually a design decision. But we're certainly open to suggestions for improvement (like ticket:1392)

[bernhard]

Hi Josh!

Sure. Wanna submit a patch?

Here's what I did:

Code: Select all

omero_dist/lib/python$ mkdir omero/sys2
omero_dist/lib/python$ touch __init__.py


Content of __init__.py

Code: Select all

from sys import modules
from glob import glob
from os.path import abspath, dirname, basename

# path to omero_dist/lib/python dir
lib_python = abspath(__path__.pop() + '/../..')

sysfiles = glob(lib_python +  '/omero_sys_*.py')

for f in sysfiles:
    # import omero_sys_* file
    m = __import__(basename(f).rstrip('.py'))
    # update this modules dict with symples from imported module
    modules[__name__].__dict__.update(m.__dict__)


Now in python I can do

Code: Select all

from omero.sys2 import ParametersI


That all the model objects are loaded into omero.model is a by-product of the ObjectFactory doing it's work, not actually a design decision. But we're certainly open to suggestions for improvement (like ticket:1392)

Well, it's fine since it works. But as said, partly irritating since I can do import omero.sys but there is no sys module or package in lib/python/omero and in particular the ParametersI is not there. The same applies to the model package, the __init__.py is empty and thus it's a bit difficult to figure out where the omero.model classes are defined.

Best wishes! Bernhard

[carlos]

Following up on your code snippets, here's a somewhat more generic way to achieve the same thing, populating the existing omero.sys module:

Code: Select all

import path
# Import omero_* implementations
load_imp = (('omero_sys_', omero.sys),)

for prefix, module in load_imp:
    for impmod in (x.basename() for x in path.path(__file__).dirname().parent.abspath().listdir() if x.basename().startswith(prefix)):
        if impmod.endswith('.py'):
            impmod = impmod[:-3]
            k = impmod.lstrip(prefix)
            # make sure we are not overriding anything
            if hasattr(module, k):
                continue
            # put the omero_sys_* implementation into the omero.sys namespace
            setattr(module, k, __import__(impmod))


This code goes into omero/__init__.py, anywhere below the import statements. Using this, if for example omero.model wasn't being pre-imported by the ObjectFactory, one could just add to load_imp:

Code: Select all

load_imp = (('omero_sys_', omero.sys), ('omero_model_', omero.model))

(untested)

I didn't run any tests other than the cli exercises, so this needs to be checked, but would something along these lines solve your issue? Please note that omero.sys.ParametersI will be the module, not the class, but that was on purpose and can be changed.

[bernhard]

Hi Carlos!

Thanks for the code. That should work. Though, from a point of a design view following the "Zen of Python - 2. explicit is better than implicit", I'd prefer having an explicit sys package folder with a dedicate __init__.py file and the same for other sub-packages.

But anyway, I don't care much as long as from omero.sys import ParametersI works as expected.

Thanks! Bernhard

[carlos]

Hey Bernhard,

I certainly agree with explicit being better than implicit, and although each of us thinks of code using our own very particular paradigm, I tend to agree with most of the best practices. Now, in this particular example, that doesn't really apply, imho... We're not dealing with a pure python approach to a problem, but rather a python wrapper, with most of the code generated from a base shared by multiple languages.

Of course we could wrap it all beautifully and pythonically, but the effort of doing so in a non automated way is just prohibitive. The solution I proposed could be used for a separate package if you feel so inclined, but it's still an added step over the generated sources, and that will always be the case here.

Anyway, change to fit your needs and, as Josh said, do submit a patch so this may land in omero's codebase.

Thanks for the feedback!

[jmoore]

You might want to give things a try, Bernhard, after https://trac.openmicroscopy.org.uk/omero/changeset/4898 and other recent commits. Unfortunately, I completely oversaw that there wasn't an `omero/sys/__init__.py`. I thought there was one, and wasn't quite sure what you were talking about. Mea culpa.

If anything thing still seems odd, post here or add a ticket. Thanks for all the help!
~Josh.