Make astropy code compatible with Python 2 and 3 without requiring 2to3 - #1219

Seems tricky without requiring Python 3.3 and its support for the u string literal prefix. (The only workaround I've seen is to define a u function). Anyone have a better solution?

Could always just add a checklist to this ticket.

Armin Ronacher suggests dropping support for Python 3 < 3.3. I don't know that I want to do that yet, so in the meantime a u() function would be needed, unfortunately. That would be easy to test out without making any other changes though, so it might be worth doing just that first and seeing if it impacts performance anywhere.

Note we can't use u() since that will be confusing since we use u internally for units - but I get what you are saying.

(cross posted from #1212)

Would it be feasible to add some hook into setup_packages.py to disable running 2to3 for individual sub-packages that are python 2 and 3 source compatible? io.ascii is pretty much there already, and I'm hoping that time and table wouldn't be too much trouble.

I bet the number of astronomers doing production work using astropy with Python 3.1 or 3.2 can be counted on one hand. Perhaps I'm wrong, but it seems like any Python 3 users are basically cutting edge and will have the latest release. Maybe we could send a poll on the astropy list, but if supporting 3.1 / 3.2 forces hacky workarounds and potential for bugs etc, it just doesn't seem worth the effort. I agree with Armin that 3.1 and 3.2 were basically DOA.

I think that's a good idea, but I think it would probably be less effort to just go ahead and convert those packages.

Problem is most Linux distros don't have 3.3 as the default Python 3 version. Most recent releases of Ubuntu and Fedora, for example, are still on 3.2 at the greatest, IIRC.

Don't forget about the six module: http://pythonhosted.org/six/ that scikit-image were using at SciPy to make their stuff cross compatible. It already defines a u() function.

BTW I stumbled on https://wiki.ubuntu.com/Python/3 which seems to be a nice reference, complementary to http://lucumr.pocoo.org/2013/5/21/porting-to-python-3-redux/.

I was just going to link to Ubuntu's guide too. I used six in porting d2to1 to be cross-version-compatible and found it to be useful. Some seem to argue that it's too big a hammer for the job it does, but for a project the size of Astropy there are enough moving parts that it might not be overkill. Also for d2to1 I support Python 2.5, so six is that much more helpful. For Astropy that's less of an issue (for example it isn't necessary to support multiple exception syntaxes).

I noticed that six.py is only 404 lines long, so it would be easy for us to include it in Astropy.

I still think that actual Python 3.1 or 3.2 use in the wild for astropy users is close to nil, and by the time we are seeing Python 3 adoption in the 10% rate that the most recent linux distros will all have 3.3. I guess it's a matter of how much cruft gets in the code and stays there forever (or until we back it out again in 5 years). I think it's better as a project to just say we support Python 3.3 and that's it.

Again harkening to Brian Granger's SciPy talk, you have to really consider the cost of adding features (in this case Python 3.1 & 2 support) in terms of the code base complexity. If we just say we only support 3.3 and dozens of users complain we could revisit, but don't try to do everything out of the box even without a demonstrated need. I would guess that more and more other packages will choose 3.3 or later so that the pre-3.3 ecosystem will shrink in size.

Just to make sure I understand, what is gained by supporting only 3.3 if we also have to support 2.6? Do you have specific examples?

Just for the record, I'm using Python 3.2 by default, since a couple of packages I rely on are 3.1 and 3.2 but not 3.3-compatible, so not willing to drop support for 3.1 or 3.2 yet.

I believe, from reading the link @taldcroft posted above, you can support 2.6,2.7 and 3.3 without requiring any nasty workarounds, certainly no need for a package like six.

Just for the record, I'm using Python 3.2 by default, since a couple of packages I rely on are 3.1 and 3.2 but not 3.3-compatible, so not willing to drop support for 3.1 or 3.2 yet.

Blast you @astrofrog! 😺

For info, the package I need is h5py, for which the latest stable version fails on 3.3. In fact, Astropy has an optional dependency on h5py, so not my fault ;)

darn it h5py... I need that as well.

Does the dev h5py work on 3.3?

there is a patch in their 2.1 branch: h5py/h5py#251

The one that impacts most of the units and wcs code is the fact that there's no cross-version way to create a unicode literal until 3.3 added back the u prefix. So you have to do hacks like: u("°"). Other than, that, in my experience, you can get a long way as long as you __future__ import a lot of things.

Right--the unicode literal prefix is the only thing really gained from 3.3. It is a big win though since using a function to wrap unicode strings is a pain, though not that much of a pain to implement since a simple tool can automate most of the work (and it's easy to remove once we drop 3.x < 3.3 support). Which is why I still say we try putting a wrapper function around unicode strings where necessary and see how it impacts performance.

If it's too bad we can put this off until 3.3 is more widely adopted and supported. Either way I wouldn't drop 3.1 or 3.2 support for Astropy 0.3. Maybe for 0.4.

This may come in handy: https://github.com/mitsuhiko/python-modernize

I think the clean solution will be to phase out 2to3 between 0.3 and 0.4 and make 0.4 support only 2.6, 2.7 and 3.3+ (as @iguananaut suggested).

Just FYI: I'm working on a solution to exclude packages from 2to3 conversion, and then will probably convert a package or two to start as a proof of concept.

Just FYI: I'm working on a solution to exclude packages from 2to3 conversion, and then will probably convert a package or two to start as a proof of concept.

👍

I'm with @astrofrog -- I use python 3.2 as my default, and have packages I use that don't work in python 3.3. The most important of which is h5py, which is perfectly happy under 3.2 but not 3.3. This has been fixed in the development version of h5py, but not the release version (scheduled for 2.2.0).

I've attached an initial stab at this that adds six.py to extern, adds a hook to the setup_package.py stuff to exclude packages from 2to3, and then has an example "conversion" for io.votable. I used io.votable since it's so "unicodey", I thought I might run into problems. However, I didn't run into anything so ugly that I was clamoring for the u prefix in Python 3.3.

It does raise some questions:

I like six.py, and it's as close to a "standard" for this stuff as it gets, which I think helps readability for others -- but it is redundant to some of the stuff in io.fits.py3compat (and possibly elsewhere) and it takes a different approach. six.py is pretty hands-off -- you have to explicitly use it -- whereas py3compat does some namespace injection here and there. I think I prefer the six.py approach as it's less magical, but we should discuss before I unilaterally include it.

I'll take a closer look at this and see what stuff I can take out of my py3compat module that would be redundant. I think as long as we're supporting Python 3 < 3.3 using six.py does buy us enough to be worthwhile, as I found to be the case with d2to1 as well. I'm not completely tied to all of the magic in py3compat but I again I need to take a closer look (some of what's in there includes monkey-patches for bugs in Numpy, for example, that probably need to hang around).

If FITS is holding this up I will work on it today, though on the PyFITS end first most likely.

No, fits isn't holding this up. I was just wondering if we're all in agreement on the general approach. If so, we can merge this and then start tackling other packages.

I agree with this approach! Using six makes sense 👍

Before this is merged, the Python 3.x failures should be tackled though.

Ok. I think I have the Python 3.2 fixes in now.

Ok - feel free to merge once the tests pass!

And on second thought PyFITS still supports Python 2.5. I need to release PyFITS 3.2 sooner or later and say that's the last one that will support 2.5. In the meantime I can just fix this in Astropy...

So, I guess if this has been merged it should be included in 0.3?

Yes, 0.3 makes sense. I like this approach, since it means we don't even have to have all modules working with six by 0.3, right?

That's right -- we don't have to do everything at once. I wanted to get this in so I and others can start converting packages as we have time.

Maybe we should reopen this discussion about supporting 3.1 and 3.2, prompted by #1398? Previously the main objection to dropping 3.2 was support for h5py in 3.3. Now h5py version 2.2.0b1 is released (since July 2013) with python 3.3 support. Just looking at the diffs in markupsafe which restore 3.2 compatibility makes me again say that astropy should just focus on >= 3.3 in order to have a clean codebase that is reasonable to read.

(OK, h5py 2.2.0b1 is beta, but they have it on their main download page so they seem to consider it solid).

Drat, I just (optimistically) checked the Anaconda distribution and for Python 3.3 they have neither h5py nor pytables. This is a little surprising since at least pytables 3.0 claims support for Python 3.x, which presumably includes 3.3.

I chose to convert io.votable first because it's so "Unicodey". It actually wasn't that hard to support 3.2 after all, so I'd not worry too much about continuing to support it for the time being.

@mdboom - OK.

Plus there's still the issue that many Linux distros are still on 3.2. I converted astropy.io.fits which also didn't have too many unicode issues after all either.

I've actually switched over to python 3.3 as my primary distribution, and there are still a few warts relative to 3.2 -- the kind of warts that mean to install all the things I want (like matplotlib) I have to do some source code hacking on various packages (not astropy).

For better or for worse, python 3.3 is a lot more sensitive to encoding issues than 3.2 (or 2.x). This results in
what seems to be a common problem where setup.py reads some file that contains non-ascii characters (often somebodies name) ignoring encoding, and so if your default encoding is ascii, you can't install the package. In particular, python-dateutil doesn't install on 3.3 unless you are comfortable changing your default encoding or hacking setup.py. That doesn't seem so interesting unless you happen to recall that matplotlib-1.3.0 requires python-dateutil. So -- at the moment -- going to 3.3 may require more than we feel comfortable asking of users.

PS: Yes, I have been submitting patches to fix this where I run into it, but python-dateutil seems to be verging on abandonware. Which means I have no idea when this will get fixed.

Hmm. dateutil is a useful enough module that maybe it would be worth forking and making into a more "active" project.

@aconley: As the maintainer of matplotlib, I'm not aware of any specific problems on 3.3 (it passes its unit tests there, at least). And the latest version of dateutil on pip (version 2.1) installs fine for me on 3.3. Can you report an issue on github.com/matplotlib/matplotlib?

It isn't a problem with matplotlib -- it's purely with python-dateutil. So is filing an issue for matplotlib really appropriate?

And it depends on your encoding -- which probably means your default isn't ascii.

 import sys
 print(sys.getdefaultencoding())

If it's utf-8 or something, you shouldn't have the problem I am having.

@aconley -- Indeed there's maybe not much we (matplotlib) can do, but it's still good to know that one of our important dependencies is having this issue. I'll see if my poking at dateutil helps any.