More import cleanup

[robertwb]

Importing all from __init__.py simplifies and stabilizes the import order. Unfortunately the current state of imports depends on packages being partially loaded at different points in time, but this patch changes things where it can without breaking sage.

[robertwb]

Generated with

import re
import ast
ignore = re.compile('(file.*no.*empty)|(no.*empty.*file)')

def add_import_all(source, pkg):
    if 'import all' in source:
        return source
    lines = []
    for line in source.strip().split('\n'):
        if line and line[0] == '#':
            if line.strip('#').strip() in ('', pkg) or ignore.search(line[1:]):
                continue
        lines.append(line)
    if lines:
        lines.append('')
    lines.append('import all')
    lines.append('')
    return '\n'.join(lines).lstrip()

def import_all(sage_root):
    for root, dirs, files in os.walk('%s/devel/sage/sage/' % sage_root):
        if '__init__.py' in files and 'all.py' in files:
            file = os.path.join(root, '__init__.py')
            source = open(file).read()
            try:
                new_source = add_import_all(source, os.path.basename(root))
                if new_source != source:
                    ast.parse(new_source)
                    print file
                    open(file, 'w').write(new_source)
                    r = os.system("%s/sage -b > /dev/null 2> /dev/null" % sage_root)
                    if r == 0:
                        r = os.system("%s/sage -python -c 'import sage.all'" % sage_root)
                    if r != 0:
                        print "Failed!"
                        open(file, 'w').write(source)
                    else:
                        print "Good!"
            except Exception, exn:
                print file, exn

[mderickx]

Cool, could you explain (maybe with a small example if that is not a crazy amount of work) what the precise difference is this makes for importing. I might be ably to figure it out on myself, but since you probably already know it , I don't want to waste my time on thinking to much about the subtle differences between our recursive all.py structure, and just doing import all from init.py?

Or is there really not much of a difference and is this just a first step to better use of init.py?

[robertwb]

Sorry I didn't see your comment earlier.

Basically, when you do import a.b.c it first loads a/__init__.py, then a/b/__init__.py, and finally a/b/c.py (or a/b/c/__init__.py).

Importing all imposes no restraint, so one can import sage.rings.foo.fragile_module without loading sage.ring.all. If fragile_module depends on stable_module being loaded first, then sage.rings.all can enforce this order, but only if sage.rings.all is loaded before import sage.rings.foo.fragile_module. Even when things aren't fragile, this ensures that the submodules of sage.rings get loaded in the order specified in all.py, greatly reducing the chance that a change in imports in an unrelated portion of code can trigger a chain reaction, import things in a different order, and have result in circular import errors. This will also help with performance, as one can see the actual load time of sage.rings.all without wondering about how many submodules were "preloaded." So it's both a cleanup and a prelude to further cleanup.

[jhpalmieri]

Would something like this also make sense, in some cases:

sage/algebras/all.py

@@ -29 +29 @@
 from free_algebra import FreeAlgebra, is_FreeAlgebra
 from free_algebra_quotient import FreeAlgebraQuotient
 
-from steenrod.all import *
+from steenrod import *
 
 from group_algebra_new import GroupAlgebra

and then just rename sage/algebras/steenrod/all.py to sage/algebras/steenrod/__init__.py?

[robertwb]

Yes, but that would involve changing a lot more code (and people's behavior).

[mderickx]

Thanks for the explanation. It seems that this will indeed make the import order more stable, so from a logical point of view I give it a positive review. Luckily the patch still applies without errors to the just released sage-4.7.2. I'm now running all doctests just to be sure.

[mderickx]

All test passed on sage.math!

[mhansen]

Replying to jhpalmieri:

Would something like this also make sense, in some cases:

sage/algebras/all.py

@@ -29 +29 @@
 from free_algebra import FreeAlgebra, is_FreeAlgebra
 from free_algebra_quotient import FreeAlgebraQuotient
 
-from steenrod.all import *
+from steenrod import *
 
 from group_algebra_new import GroupAlgebra

and then just rename sage/algebras/steenrod/all.py to sage/algebras/steenrod/__init__.py?

Also, for things like "sage.plot.plot" it could be confusing since if you just did "sage.plot.plot" it would be referring to the function (currently, "sage.plot.all.plot"). However, to import that function directly, it'd be "from sage.plot.plot import plot" which would also be the same as "from sage.plot import plot".