search

weaverba137 / pydl

Library of IDL astronomy routines converted to Python.
BSD 3-Clause "New" or "Revised" License
28 stars 16 forks source link

Fix FutureWarning due to nested sets in re in Python 3.7 #44

Closed albireox closed 5 years ago

albireox commented 6 years ago

In Python 3.7, the re module raises a FutureWarning when using nested sets in a way that is not compatible with the syntax that will be used for them in the future. See here and here.

The yanny module was working fine with Python 3.7 but throwing a series of annoying FutureWarning messages. This seems to fix it.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 62.869% when pulling fe01c4a51c7dcfd16042dd0f3b98acfc8cbaf20d on albireox:re-nested-sets into d7f5261ca944a71941dc6ee1ec815f29cbe8bdaf on weaverba137:master.

weaverba137 commented 5 years ago

Thank you for this PR, I've been a bit busy lately. Could you please add a Python 3.7 test to .travis.yml (see line 64) and update docs/pydl/changes.rst?

albireox commented 5 years ago

Done and passes.

I tried adding a test

        - os: linux
          env: PYTHON_VERSION=3.7 NUMPY_VERSION=1.12

but seems to fail (https://travis-ci.org/weaverba137/pydl/jobs/405892689). Not sure why and I don't fully understand the setup of your travis builds. I removed it but let me know if you want me to try harder.

weaverba137 commented 5 years ago

Did you look at line 64 of .travis.yml? All you needed to do was duplicate that line and change it to 3.7.

albireox commented 5 years ago

Yes, I did that. See the diff. I then tried to add a new testrun as in lines 101-102 with the text

- os: linux
   env: PYTHON_VERSION=3.7 NUMPY_VERSION=1.12

and that one failed. But I added the a line in 64 for Python 3.7

weaverba137 commented 5 years ago

Ah, OK, I see what's going on now. I recommend using a more recent version of numpy for the test.

albireox commented 5 years ago

Right. Using numpy 1.14 made it work. There are a couple test builds that failed, but seem spurious failures. You may want to have a look at that.

weaverba137 commented 5 years ago

Indeed, one of the Travis tests was just a fluke, however, there is a problem with the documentation that needs to be fixed. Looks like a problem with the link to this PR in the changes file.

albireox commented 5 years ago

Damn, yes, I don't know what I was thinking when I added that PR link. It seems to work now.