discos / basie

Italian radiotelescopes scheduling software
BSD 3-Clause "New" or "Revised" License
0 stars 2 forks source link

[WIP] Python 3 compatibility #41

Closed matteobachetti closed 6 years ago

matteobachetti commented 6 years ago

Lots of changes. All tests pass on Python 2.7 and 3.6 on my machine and on Travis. Is 2.6 compatibility a must-have?

Modifications in this PR:

There is a single modification I am not too confident in. There is a failure when calling

    res = conf.validate(rich_validator, preserve_errors=True)

in rich_validator.py, function validate(). Astropy tries to import validate and it fails (probably related to #29 ?). I set preserve_errors=False, and tests keep passing. I guess the error reports won't be verbose enough if we do that? What other consequences do you expect?

UPDATE: It is actually fixed in recent astropy versions (> Jun 2018), and I opened a PR to backport the fix to earlier versions: https://github.com/astropy/astropy/issues/7802

Hope this works!

flyingfrog81 commented 6 years ago

Hi @matteobachetti ! It is nice to see someone taking care of this little and deformed creature ;)

Compatibility with Python 2.6 is there because of the Python version that is running on ACS control system machines if I remember correctly. It would be entirely possible to have a separate Python 2.7 or Python 3.0 environment compiled and installed on those machines so that we can get rid of 2.6 compatibility but this involves some more work on those machines.

I trust you on the usage of astropy.extern , I have never used it before.

You should not change the preserveError flag, if I remember correctly it has implication on nested validation functions and, as you correctly say, on error messages, which are quite important to the user indeed (I can't remember how many times I've been asked to change those to something more significant) .

A quick and dirty hack could really be to have a lib folder with the sources of configobj and validate included, and imported from the local path.

matteobachetti commented 6 years ago

@flyingfrog81 Done :) I called the new module basie.configobj. Is there a strong reason to call it lib?

flyingfrog81 commented 6 years ago

Well no, no real reason to have it under a lib directory apart from following a common naming scheme that you can often find in software distribution. It is not a rule, but sometimes when you embed external libraries in your distribution you want those collected separately under a lib folder. Having said that, any other place would do.

matteobachetti commented 6 years ago

@flyingfrog81 now tests pass under Python2.6 too. I had to fix the versions of a few libraries, but the code itself works.

Since python 3 compatibility is a large step, and there is some testing to do, I advanced the version to 0.7b1 (so that it doesn't get installed automatically by pip). Let me know if it's the correct choice

matteobachetti commented 6 years ago

Also, I notice the url in setup.py is "http://github.com/flyingfrog81/basie/". Should I change it to "http://github.com/discos/basie/"?

flyingfrog81 commented 6 years ago
matteobachetti commented 6 years ago

Speaking with Sergio, it seems that 1.0b1 is indeed an option. I will change it to that, so that the next version of DISCOS ships with the 1.0 version of Basie.