Remove python 2.7 support (fix #498) #665
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 tasks
The sub-classing of the Mock object may still be useful, even though it was added in the hope of getting a Python 2.7 documentation build. This should be checked to see if it can be removed.
The setup.py script now fails if run with a python older than 3.5. The package metadata has been updated to reflect that Sherpa is now 3.5 or later (with python_requires set to not include Python 4 or later), so that pip installations will only work with a supported Python version. It is not clear from https://packaging.python.org/guides/dropping-older-python-versions/#dropping-a-python-release whether we really should have included python_requires in the 4.11.1 release, but I think we are okay as is. The Travis tests should fail because I have not removed the python 2.7 build.
|
The above commit - 47cf063 - should fail for the python 2.7 build on travis (but not the other builds). edited to add good, I see the expected error message from the 2.7 build and it is also the only test that fails. |
I have bumped the NumPy version to 1.16 from 1.11 as we have 1.15 well tested, and it doesn't seem to make much sense to use older versions of NumPy. An alternative would be to remove this bare-bones test.
The code conversion was pretty much mechanical. I converted 'from six imprt string_types' to 'string_types = (str, )' to make it easier to see where this was happening (in part because we probably should review all these uses to see if there are better changes). There are also several minor "python 2.7 clean ups" in this commit, e.g. replacing 'try: python27-code except: python3-code' by python3-code. There conda recipes still include six.
Python 3 now comes with mock, so we no longer need to install it for testing. There were several places where both the Python 2.7 and 3 mock modules were imported which have been cleaned up.
This is not needed with Python 3+.
This updates the test that used to be Python 2.7 from NumPy 1.16 to 1.17.
|
Apparently conda doesn't have numpy 1.17 yet - perhaps I should have checked before pushing 135d0f2! Oh well, it can be rebased out of existence. |
wmclaugh
approved these changes
Aug 27, 2019
wmclaugh
reviewed
Aug 27, 2019
sherpa/models/template.py
Outdated
| @@ -1,6 +1,6 @@ | |||
| from __future__ import absolute_import | |||
| # | |||
| # Copyright (C) 2011, 2016 Smithsonian Astrophysical Observatory | |||
| # Copyright (C) 2011, 201, 20196 Smithsonian Astrophysical Observatory | |||
There was a problem hiding this comment.
typo in copyright dates- should be 2011, 2016, 2019
There was a problem hiding this comment.
Oops. Thanks for the update - was in a L3 meeting.
|
@wmclaugh - do you want me to rebase this to avoid the "update to numpy 1.17, downgrade to 1.16" step, or are we happy to keep this in? |
|
@DougBurke - I figure it would be easier for me to just revert the
travis.yml file from my PR and merge yours as is.
…On Fri, Aug 30, 2019 at 10:24 AM Doug Burke ***@***.***> wrote:
@wmclaugh <https://github.com/wmclaugh> - do you want me to rebase this
to avoid the "update to numpy 1.17, downgrade to 1.16" step, or are we
happy to keep this in?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#665?email_source=notifications&email_token=ADF7K6F6F3GNI3B6B2C3KDDQHEUSRA5CNFSM4IOJMWVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5RZWIA#issuecomment-526621472>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADF7K6ALCZZ7D4ZDMWOVNO3QHEUSRANCNFSM4IOJMWVA>
.
|
|
@DougBurke-
I realized what you meant after I sent the response. I am not overly
concerned with the history and am fine merging the PR as it stands.
…On Fri, Aug 30, 2019 at 12:06 PM Doug Burke ***@***.***> wrote:
Sorry @wmclaugh <https://github.com/wmclaugh> I wasn't clear enough. This
PR contains 135d0f2
<135d0f2>
and 337b26c
<337b26c>
which cancel each other out. I could rebase to remove these (and also take
your copyright fix into an earlier commit), which gives a slightly-cleaner
history, but do we care that much?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#665?email_source=notifications&email_token=ADF7K6DGEJHOSNS4TUKS62TQHFASDA5CNFSM4IOJMWVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5SCVJI#issuecomment-526658213>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADF7K6DYKWPGEX5YXPGLILLQHFASDANCNFSM4IOJMWVA>
.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Remove support for Python 2.7 from Sherpa. The metadata used by pip now requires python 3.5 or higher (but not version 4), and an attempt to build with an earlier version of python will cause
setup.pyto exit with an error message. The documentation has been updated, including a link to the final Python 2.7-compatible release (Sherpa 4.11.1). The code has been cleaned up to remove the use of the six module (that provided backwards-compatible support) and other parts of the code that supported running under Python 2.7 and 3.5-or-later.There has been no attempt to re-write the code to take advantage of capabilities present in Python 3.5 that were not in Python 2.7.
Details
Remove Python 2.7 support in Sherpa.
README.mdsetup.pyobjectbase class from objectsOne problem with testing the six removal is that it is still required by other modules we use, so it is currently hard - if not impossible - to ensure we have a "six free" environment.
I may make some more minor clean ups to address some of the Python 2.7 clean ups, but will not touch the conda recipies.
Questions
The metadata in
setup.pyhas been updated to point out that Python 3.5 or later is needed (and does not support a hypothetical python 4). This means that 'pip' should not try to install it. However, it doesn't stop someone from trying to 'python setup.py install' a source-code installation, so I added an explicit check for that - you can see the output in #665 (comment)Is this a sensible thing to do?