Issues with vo legacy shim - #193

The second part of the issue is resolved by the attached pull request. It seems that if you import different modules using imp.load_module using the same name, it ends up using the same namespace across calls.

The first part is trickier. I was compelled to change the relative imports that go outside of the io.vo tree to absolute imports, i.e.:

from .. import fits

to

from astropy.io import fits

but that seems to break the python setup.py test in the common case. Will spend some more time looking into this.

These fixes work for me for the multiple install and the logic issues.

I haven't tested this yet, but is the issue with relative imports in compatibility packages still unresolved?

It kind of seems strange to me, like it shouldn't do that. But I'll have to look into it more. But if there's no other way, maybe this is a good excuse to call for reneging on the relative imports requirement? :)

Here's the issue here -->

It's easy enough to get at things that are imported by __init__.py -- the shim just needs to do:

from astropy.io.vo import *

However, in order to import modules of a package from outside that package, eg.

from vo.table import parse

the shims do

import pkgutil
__path__ = pkgutil.extend_path(__path__, "astropy.io.vo")

This sets up the shim "as if" it were at astropy.io.vo, but it in that situation, it doesn't allow relative imports that go above that level (in this case astropy.io.vo).

I tried changing all of this "peer" imports to absolute ones, but that breaks python setup.py test (I think just because it gets confused and tries to load astropy from the source tree rather than the built tree.

There's probably a way out of this mess, just haven't found it yet (and also haven't devoted much time yet).

As I was fiddling around to try and understand this better, I noticed that the following works:

import sys
from astropy.io.vo import table, tree
from astropy.io import vo
sys.modules['vo.tree'] = tree
sys.modules['vo.table'] = table
sys.modules['vo'] = vo

del table, tree, vo

from vo.table import parse
from vo.tree import Table, Field
import vo.table as t
import vo

parse('aj285677t3_votable.xml', pedantic=False)

I checked, and vo.__path__ points to astropy-0.0.dev991-py2.7-macosx-10.6-x86_64.egg/astropy/io/vo as expected... Am I missing something?

@astrofrog: The problem with such an approach is that it requires every module that could be imported to be imported in advance. This will not be ideal for subsidiary functionality (e.g. plotting) with extra requirements etc. Whatever approach we take needs to be "lazy".

I believe I have a solution now based on PEP 302 finders and loaders that does the mapping from vo.table to astropy.io.vo.table on the fly. It seems to work well in the cases I've tested (from vo.table import parse, from vo import table, import vo, from vo import *). Can you confirm it works for you as well?

There is a theoretical downside in that:

>>> from vo import table
>>> table.__package__
'astropy.io.vo'

which is surprising, but I don't know of any code that relies on __package__.

Finally getting back to this. For some reason GitHub hasn't been working for me on my work computer for several days. IT seems to have fixed it, though I still need to find out what the problem was...

Anyways, this solution looks great. As for the __package__ issue, my understanding is that it has to be what it is in order for relative imports to work correctly... So yes, surprising. But you're right that it's unlikely for there to be any client code that uses it.

Just so we're clear, @mdboom, is this ready to merge once @astrofrog confirms that it fixes the original issue? It appears to do the trick for me, but it would be good to confirm with @astrofrog, as well.

Also, I just noticed that if I use python setup.py develop instead of python setup.py install, the legacy shims don't get installed (or at least from vo.table import parse does not work). Is that expected behavior?

Yes, I think this is ready to merge.

I'm not sure how to support python setup.py develop. develop seems to be based on the assumption that the installation heirarchy mirrors in the installed heirarchy, which makes doing this stuff kind of difficult. We could detect develop and dump the files at the root of the source tree, but I'm really not a fan of packages that do that -- source and build products should be kept separate as much as possible.

It seems to work for me!

Yeah, I really don't think they need to be supported for develop.

Yeah, I'm fine with not supporting develop mode for this sense - develop is a convinience for certain things, but it shouldn't be the canonical case. Just wanted to make sure.