Code is both Python 2.7 and 3.5 compliant

[olaurino]

Description

Resolve #76.

Introduce support for Python 3 while keeping code base compatible with Python 2.7.

Introduced a Python 3.5 travis build, but we can target more Python 3.x versions later.

This PR also tries and introduce new unit and integration tests.

As a side effect, this PR also fixes #186.

[DougBurke]

@olaurino are you looking for comments at this stage, or is more work needed before a review is needed?

[olaurino]

Comments are always welcome, especially in this case where the PR is going to be massive. You already caught an unintended change, so that's good :)

[DougBurke]

Is the idea here that, because the only use of the stk module is already within a try/except block, there is no need to create a "replacement" routine for stk.build() in the except clause? I thought that the change looked odd, until I checked into how the stk module was used here. Do we need to document this (e.g. in a comment) for anyone who wants to use stk,build in this module? Or am I being over sensitive?

[olaurino]

That is exactly what I had in mind. Yes, we could improve the documentation (as a matter of fact I had a comment that explained that but apparently I dropped it after changing the try..catch block). If the stack library is not present and you use the stack syntax (e.g. @filename) you'll get a file not found error, which paired with the warning should be reasonable enough.

[DougBurke]

I noted that there looked to be similar-ish logic in the sherpa/astro/datastack/utils.py module. I didn't look closely enough to see if the code was close enough to be able to remove one of them. That is, set Datastack.load_pha to load_wrapper(ui.load_pha).

Perhaps there's something obviously different that I'm just missing (as I'm only skimming through the code).

[olaurino]

You are absolutely right, I noticed that too. I have the temptation to fix things like this as I go along, but I don't want that to be in the way of the python 3 migration, and the clock is ticking. There does not seem to be any reason for the duplication, but there is some complexity in the way the wrapping is done, so that might hide a trap... or I just forgot to remove a method when I did the refactoring for #22. Either way, I would rather spend the time required to figure this out adding test coverage where we are missing it. In any case I appreciate the level of your review, so please keep making these kinds of comments :)

[DougBurke]

I see that the python 2.7 tests are now back to passing (yay), but the stk and group tests are skipped. Is the idea to leave these for now and concentrate on the main code (which looks like replacing lots of use of izip!) and then come back to these later (in part because these are OTS from our view, so we are waiting for other teams to update them)?

[olaurino]

@DougBurke that's the idea.

[bsipocz]

This should be exactly the same as the build you want to allow to fail, so here I guess:

     - env: XSPECVER="12.9.0i" FITS="astropy" INSTALL_TYPE=develop TEST=submodule MATPLOTLIBVER=1.5
       python: "3.5"
       sudo: required
       dist: trusty

[olaurino]

Thanks @bsipocz. Not sure why I kept the allow_failures line, since I am happy with the build to be red until all the tests pass. I'll remove that line altogether, rather than fixing it.

[DougBurke]

I think, once we've got the python 3.5 travis builds green we should add in tests for python versions 3.3. and 3.4. Ideally we'd be able to support all versions, but if not, it would be good to know which ones we don't support.

[DougBurke]

@olaurino - I've played around with a few fixes but you may not want them (I am just relying on the existing tests rather than checking if new tests are needed). You can find them at

https://github.com/DougBurke/sherpa/tree/python3

and they are basically StringIO, basestring, and xrange fixes.

edit I've added a few more: I was focussing on improving the serialize tests, which lead to getting sherpa.astro.io to import (but not much more, since now the errors are getting more arcane) and also remove the inspect.getargspec deprecation warnings (this last one adds some tests, so is probably worth looking at).

[DougBurke]

@olaurino - OOPS. I did not mean to push those changes! I'm not sure what the best way to fix this is, so I'm going to leave it for now.

[olaurino]

@DougBurke you can leave them here, thanks! I wonder where the conflicts are coming from, I'll look into that and rebase.

[DougBurke]

I've just added PR #250 which removes the capsulethunk file. It was surprisingly easy (yay for encapsulation, I guess), so maybe I missed something?

[DougBurke]

Should we note that this has been deprecated by #252 (or close this in favor of 252)?

[olaurino]

I am about to merge this and rebase #252!

[cdeil]

Thank you!!!

I'll add a Python 3 build for Gammapy in the next days.

Will there be a Python 3-compatible Sherpa conda package for testing soon, or only after the next stable Sherpa release?

[olaurino]

@cdeil the conda package will most likely come in a few weeks with the 4.8.2 release. However, the conda recipes are included in this repo, so you could produce them yourself. They may require bumping versions and other adjustments to the dependencies every now and then (in this case you certainly need to update the python version), but if you can build Sherpa from sources, it should be a straightforward process. If that's not true, or you have issues or questions, please let me know.

I have quite a packed schedule, but if I find some time I'll publish some test cut of the python 3 conda package. I just can't promise, so please don't hold your breath 😉

[olaurino]

By the way, I created a sherpa 4.8py3 conda package (with the dev label) about a month ago to support https://github.com/astropy/saba testing. It may be out of date, and it's Linux only:
http://anaconda.org/sherpa/sherpa/files

[taldcroft]

Congratulations on this tremendous accomplishment!! 🎉 This removes the last real blocker from Chandra operations tools migrating to Python 3.