web2py uglyness

[mdipierro]

Something always bothered be and resulted in lots of uglyness. I
suspected there was a way to fix it but did not know. Now I found out.

The problem:
==========

when we do

import a.b.c as d

Python (and web2py) look in sys.path.
If the module is in "web2py/applications/yourapp/modules" then
"web2py/applications/yourapp/modules" should be added to sys.path.
This would cause a major problem if there are many web2py apps that
have a file a/b/c.py in modules. The import would find the first one,
not necessarily the one in the current app.
sys.path is not thread safe. There only a global sys.path not one per
thread.
So far the suggested solution was not do add the app modules path to
sys.path and instead we used to do:

exec('import applications.%s.modules.a.b.c as d' %
request.application) # UGLY!

This solves the conflict between app of modules but not conflicts with
modules that are in sys.path.
This limits which modules can go in the app modules/ folder because
modules that user absolute imports cannot find their dependencies.
This does not reload modules and one is forced to user conditional
reloads when debugging modules.

The solution
=========

I found and implemented a better way. With the code in trunk we can
now do:

d = local_import('a.b.c')

- it is not based on exec
- it searches in applicaitons/currentapp/modules/ before searching in
sys.path so no conflicts ever
- each modules/ folder acts like its own site-packages and you can put
any third party module in there whether or not is uses relative
imports
- you can ask it force reloading modules at every request, great for
debugging modules:

d = local_import('a.b.c', force=True)

This opens the door to better plugins implemented (partially) as
modules.

Please check it out and report any success/failure. If ok it will be
in 1.70.1

Massimo

[Yarko Tymciurak]

...you can review (and comment on) this in gluon/colmpileapp.py...

In general, this is an important step towards one kind of application specific plugin (application activated, as compared to available at each application request, or as part of gluon-level activation for availability to any/all applications)....

Test away!

[Timbo]

I like the idea and I am very much against ugliness...however:

d = local_import('a.b.c')

is way uglier in my book than:

import a.b.c as d

Asthetics aside, it does not solve the original problem very well.
The problem is not being able to do:

import c as d

when d is in apps/init/modules because sys.path is global.

A much better solution is to change local_import to be just this:

d = local_import('c')

Because you are in an app the 'a.b' part is assumed.

Also, I like the ", force=True" part, but call it "reload" instead
since that better describes what it does, otherwise, "what the heck
are we forcing?" is a valid question.

If you need a patch for this, let me know. I don't mind.

Just my 22222c,
-tim

[Timbo]

Disregard the first part of my original post. I've looked at your
code and you've done the logical thing. But your original email did
not state it properly.

For the record of those reading:

If you have a module that you want to use in applications/<appname>/
modules, with this new addition you can:

mod_name = local_import('mod_name')

and it will pull it in as "import mod_name" would if applications/
<appname>/modules were added to sys.path.

For Massimo, I still think you should switch the "force" param to
"reload" for clarity reasons.

-tim

[DenesL]

+1 rename 'force' to 'reload'

[Yarko Tymciurak]

On Fri, Oct 30, 2009 at 8:20 AM, DenesL <dene...@yahoo.ca> wrote:

+1 rename 'force' to 'reload'

+1 on this rename from me also.... it's just clearer, more explicit, "self documenting"

One of my pet-peeves are web2py checkins that say "fixed bug".... ugh!  I should accelerate getting us to google code and mercurial, so we can have code reviews prior to checkin;  that would be my first set of review comments  ;-)
 

[mdipierro]

reload is a reserved keyword but ok. I will change this.

On Oct 30, 9:02 am, Yarko Tymciurak <resultsinsoftw...@gmail.com>
wrote:

[Yarko Tymciurak]

On Fri, Oct 30, 2009 at 9:44 AM, mdipierro <mdip...@cs.depaul.edu> wrote:

reload is a reserved keyword but ok. I will change this.

?  reload() i s a Python builtin function ... which does what you describe this as doing. 

I think this is consistent, but more importantly - a "word" that says more clearly what this does, and which minimizes confusion.

I for one do not think this particular parameter name would confuse anyone.

What do others think?
 

[mdipierro]

ERRATA:

After more tests

[....]

> I found and implemented a better way. With the code in trunk we can
> now do:
>
>    d = local_import('a.b.c')
>
> - it is not based on exec

actually I when running on GAE the first version did not work. on GAE
it uses execfile.

> - it searches in applicaitons/currentapp/modules/ before searching in
> sys.path so no conflicts ever

Do not use it to search sys.path because there are some unresolved
issues

> - each modules/ folder acts like its own site-packages and you can put
> any third party module in there whether or not is uses relative
> imports

I was wrong. This still does not work if the imported module does not
use relative imports.

> - you can ask it force reloading modules at every request, great for
> debugging modules:
>
>    d = local_import('a.b.c', force=True)

or "reload" instead of "force" now in trunk.

Not quote as perfect as I had hoped but it does most of what expected.

If only there was a way to have a thread level sys.path ... perhaps
there is but I do not know howto.

Massimo

[mdipierro]

ERRATA^2

The uglyness is back, just encapsulated in local_import (same syntax
as proposed).
The problem is the orginal implementation does not handle __init__.py
properly.
Doing so would be too expensive.

Massimo

[Alex Fanjul]

force_reload? its self explanatory.

El 30/10/2009 14:20, DenesL escribió:
> +1 rename 'force' to 'reload'
>
> >
>
>

--
Alejandro Fanjul Fdez.
alex....@gmail.com
www.mhproject.org

[Yarko Tymciurak]

...that is self-explanatory (I think 'reload' is too)...

[Álvaro Justen [Turicas]]

Hey, __import__ does this! What do we need a new function?
See:
In [1]: import matplotlib.pyplot as plt

In [2]: plt
Out[2]: <module 'matplotlib.pyplot' from
'/usr/lib/python2.5/site-packages/matplotlib/pyplot.pyc'>

In [3]: my_new_plt = __import__('matplotlib.pyplot', fromlist='pyplot')

In [4]: my_new_plt
Out[4]: <module 'matplotlib.pyplot' from
'/usr/lib/python2.5/site-packages/matplotlib/pyplot.pyc'>

>   d = local_import('a.b.c', force=True)
>
> This opens the door to better plugins implemented (partially) as
> modules.
>
> Please check it out and report any success/failure. If ok it will be
> in 1.70.1
>
> Massimo
> >
>

--
Álvaro Justen
Diretor de Desenvolvimento
Peta5 - Nós fazemos TV digital
http://www.peta5.com.br/
21. 3021-6001 / 9898-0141

[mdipierro]

I do not think it does what I originally wanted to do (specify the
folder where to import to) but you are right that we can rewrite the
current local_import using __import__. WOuld you send me a patch?

On Nov 3, 5:46 am, Álvaro Justen [Turicas] <alvarojus...@gmail.com>
wrote:

[Iceberg]

Tested, it works! By the way, here are some more examples I tried and
sharing now.

#1: automatically set the reload flag
MyModule = local_import('my_module',
# Treat client from localhost as a non-production environment
reload='localhost'==request.env.http_host.split(':')[0]
)

#2: how to import a class rather than a module
MyClass = local_import('my_module').MyClass