Skip to content

lapack_testing.py: python2 explicit in hashbang #489

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

Merged
merged 8 commits into from
Feb 20, 2021

Conversation

christoph-conrads
Copy link
Contributor

There is no python executable in Ubuntu anymore. From Debian package python-is-python2 description:

In Ubuntu, all python packages use explicit python3 or python2 interpreter and do not use unversioned /usr/bin/python at all. Some third-party code may still be python2 based, yet may use /usr/bin/python.

This is a convenience package which ships a symlink to point /usr/bin/python interpreter at the current default python2. It may improve compatibility with obsolete 3rd-party software, whilst breaking some modern software.

This package will be installed upon upgrades to Debian 11 (bullseye) and Ubuntu 20.04, if the DEPRECATED python2 was installed.

@martin-frbg
Copy link
Collaborator

wouldn't this be even more likely to break on systems that have only python3 installed, with a symlink to plain "python" ? AFAIK python2 itself is basically abandoned now.

@codecov
Copy link

codecov bot commented Feb 13, 2021

Codecov Report

Merging #489 (a51f8ba) into master (47a4a75) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #489   +/-   ##
=======================================
  Coverage   83.33%   83.33%           
=======================================
  Files        1820     1820           
  Lines      170857   170857           
=======================================
  Hits       142384   142384           
  Misses      28473    28473           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 47a4a75...a51f8ba. Read the comment docs.

@langou
Copy link
Contributor

langou commented Feb 13, 2021

Here is what I read on my Mac when I type "python2"

% python2

WARNING: Python 2.7 is not recommended.
This version is included in macOS for compatibility with legacy software.
Future versions of macOS will not include Python 2.7.
Instead, it is recommended that you transition to using 'python3' from within Terminal.

Python 2.7.16 (default, Jun 5 2020, 22:59:21)
[GCC 4.2.1 Compatible Apple LLVM 11.0.3 (clang-1103.0.29.20) (-macos10.15-objc- on darwin
Type "help", "copyright", "credits" or "license" for more information.

@martin-frbg
Copy link
Collaborator

@langou
Copy link
Contributor

langou commented Feb 14, 2021

We should probably either leave python or have python3, but not an explicit python2. After reading a little bit about this, I am in favor of an explicit python3. Comments, opinions welcome.

@ilayn
Copy link
Contributor

ilayn commented Feb 14, 2021

Python 2 is decommissioned and almost no OS will provide in the near future hence this will become unusable very soon if python2 is used. I don't think python version is important here anyways since it is a quite straightforward task. The python file can be improved slightly to make it less hardcoded let me know if I can be of use.

@weslleyspereira
Copy link
Collaborator

What about leaving /usr/bin/env python and adding a local symlink to the default Python.
Suggestion:

env_python="$(which python)"
if [[ "$env_python" == "" ]]; then env_python="$(which python3)"; fi
if [[ "$env_python" == "" ]]; then env_python="$(which python2)"; fi
ln -s "$env_python" python

This could avoid problems to old systems that don't use python3. Altough I don't know where to trigger such a script.

@christoph-conrads
Copy link
Contributor Author

christoph-conrads commented Feb 17, 2021

This could avoid problems to old systems that don't use python3.

Python 3.0 was released in 2008 and Python 3.5 already reached its end of life in September 2020, eight months after Python 2. CentOS 7 is the distribution with the oldest packages that I am actively using and even this distro has a Python3 interpreter in its stable package repositories.

@christoph-conrads
Copy link
Contributor Author

Python 2 is decommissioned and almost no OS will provide in the near future hence this will become unusable very soon if python2 is used.

Changing the hash bang to python2 is the easiest change but converting to Python3 is the future-proof approach (I did not check if the script is compatible with Python3).

@langou
Copy link
Contributor

langou commented Feb 17, 2021

My opinion: (1) in favor of writing python3 as opposed to python. (2) Quite against python2. (3) fine with python, preference for python3.

Quick survey:
Thumbs up for python3
Hooray for python (current state)
Heart for python2

@langou
Copy link
Contributor

langou commented Feb 18, 2021

Let us move to python3 then. @christoph-conrads : can you modify your PR from python2 to python3? I'll merge right after. Another solution would be to close this PR, and then @weslleyspereira or me can submit a new one, but might be better if we follow up on this one, and merge from this one. Either way is fine though. Let us know @christoph-conrads . Cheers, J.

@christoph-conrads
Copy link
Contributor Author

@christoph-conrads : can you modify your PR from python2 to python3?

Yes, it will be pushed later today. The changes will include

  • the hashbang update,
  • removal of the second line with the coding information (Python3 code is always UTF-8 encoded), and
  • removal of the __future__ import.

Use `python2` explicitly in the hashbang instead of just `python`
because Ubuntu 20.04 does not ship with a `python` executable any
longer.
The `-x` flag was never a valid short option for the `getopt` command
line parser.
The getopt command line parser did not expect arguments after the
`--bin` and `--dir` options.
Remove C-isms:
* remove unnecessary braces in "if" conditions
* use booleans instead of 0/1 variables
* search for a Python3 interpreter for the test summary
* replace an unnecessary macro invocation with a function
* make summary depend on all other tests

Making the summary depend on all other tests fixes test execution in
random order (e.g., with `ctest --schedule-random`).
@langou
Copy link
Contributor

langou commented Feb 20, 2021

Thanks @christoph-conrads. Looks great to me.

@langou langou merged commit edc6bc5 into Reference-LAPACK:master Feb 20, 2021
@christoph-conrads christoph-conrads deleted the python2-in-hashbang branch February 21, 2021 18:16
christoph-conrads pushed a commit to christoph-conrads/lapack that referenced this pull request May 23, 2021
…2-in-hashbang

lapack_testing.py: python2 explicit in hashbang
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

Successfully merging this pull request may close these issues.

5 participants