Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python 3 support #87

Closed
timj opened this issue Aug 18, 2016 · 16 comments
Closed

Python 3 support #87

timj opened this issue Aug 18, 2016 · 16 comments

Comments

@timj
Copy link
Contributor

timj commented Aug 18, 2016

What's the situation with Python 3? I see that there was a recent patch for print_function but there are still issues with old style exception catching and relative imports. LSST are trying to move to python 3 and I'm wondering whether you would accept patches. We are using the future package to support 2 and 3.

@dstndstn
Copy link
Owner

Hi,
Sorry for my slow reply! I would certainly accept patches, though I think I was planning to use the "six" package rather than "future"; I haven't evaluated them though. Would it work for LSST if we use six and you use future?
thanks,
--dstn

@dstndstn
Copy link
Owner

(and the situation is that we're not py3 compatible yet but would like to be!)

@timj
Copy link
Contributor Author

timj commented Aug 25, 2016

We evaluated six and future and picked future because it allows us to write idiomatic python 3 code in python 2. I'm happy to do the port to Python 3 if you let me use future. I don't have enough experience with six to do the port if you have decided to use that. I can do a modernization pass pretty quickly though to get you to python 2.7 standard (I see some string exceptions in there!).

I'm working on a LSST porting document but a summary of our position can be found at http://dx.doi.org/10.5281/zenodo.60566

@timj
Copy link
Contributor Author

timj commented Sep 3, 2016

See https://sqr-014.lsst.io for more background.

@timj
Copy link
Contributor Author

timj commented Sep 12, 2016

Thanks for merging #89. Do you have any thoughts on future vs six? If you are happy with future then LSST will be happy to finish the port (this probably means dropping 2.6 and older). If you want to use six then, as I say, LSST has no experience with it so it's not clear that we'll quickly be able to help with the port.

@dstndstn
Copy link
Owner

I think 'future' will be fine.

What are you doing about FITS i/o? I have heard horror stories about unicode/str pandemonium in many libraries, leading to very difficult bugs.

@timj
Copy link
Contributor Author

timj commented Sep 12, 2016

We haven't encountered any problems with FITS I/O so far. Maybe we've just been lucky. We have encountered fairly standard bytes/string problems with pickle and subprocess.

@dstndstn
Copy link
Owner

What FITS lib are you using?

@timj
Copy link
Contributor Author

timj commented Sep 12, 2016

Mainly cfitsio but behind a C++ swig wrapper and we did force that wrapper to use bytes.

@timj
Copy link
Contributor Author

timj commented Sep 14, 2016

@dstndstn turns out that LSST code only needs #90 from @parejkoj in order to build lsst/meas_astrom#42 so I'm afraid we can't justify completing the port to Python 3 at this time. We mainly use the C++ interface and, as far as we know, the 2.7 modernization pass in #89 was enough for us. Sorry.

@dstndstn
Copy link
Owner

Oh well, it was a good start. Thanks.

@wtgee
Copy link
Contributor

wtgee commented Oct 13, 2016

Hi all. What needs to be done with this? I'm hitting the same problems as #91 and am also pretty tired of keeping a py27 environment around simply so I can have astrometry.net work, especially when it's mostly a matter of incorrectly linked scripts rather than actual functionality.

I can take a stab at cleaning up some things so just wondering if there is an outlined TODO list somewhere. Also, it's looking like there is no test suite...(?!?).

Also, anything preventing you from using the astropy FITS I/O? (which I believe just updates @PyFITS which I believe just uses a minimal cfitsio).

@dstndstn
Copy link
Owner

I think the thing to do is (a) pull out the functionality of the various python scripts into modules in astrometry.util; (b) create new bin/script.py scripts that import and call the functions in those modules; (c) adjust makefiles to install them. It would be good to keep the current ability to run the executables from within the source directory (without having to install anything); I forget exactly what the C code calling the python scripts does to search for them.

pyfits is like someone's half-finished summer project that has been passed around and rebranded multiple times. I now vastly prefer the 'fitsio' python package.

@dstndstn
Copy link
Owner

dstndstn commented Oct 13, 2016

(and my understanding was that pyfits is pure python, not using cfitsio; but I could well be wrong)

@timj
Copy link
Contributor Author

timj commented Oct 13, 2016

I believe pyfits is mostly python but they blanched at handling the data compression part of the standard in native python so do punt that to cfitisio.

@dstndstn
Copy link
Owner

Updating this old ticket: we've made a bunch of progress on Py3 compatibility. The travis build works in Py2 and Py3 (and gcc and clang). A few specific things are now marked for py3 incompatibility, so if you try with py3 and something breaks, please file a specific ticket. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants