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

Drop support for Python 3.6 and lower #223

Open
wants to merge 48 commits into
base: master
Choose a base branch
from

Conversation

jpwhite3
Copy link

@jpwhite3 jpwhite3 commented Jun 20, 2021

As several have noted in issues, not supporting Python 3+ has been an issue. Given that the Python Software Foundation has officially ended support for the 2.x line, I though it time to address the issues by dropping support for anything 3.5 or lower. Here are the highlights of the change-set:

  • Characterization test suite has been created. Robust functional test suite is still needed.
  • All code has been formatted and linted using a moderately strict set of pylint/flake8/black rules.
  • A continuous integration process has been setup, leveraging GitHub Actions to automate testing, artifact generation, and release.
  • CI configuration tests every commit against Python 3.7, 3.8, 3.9, 3.10 in parallel.

@adam-waldenberg
Copy link
Member

Thank you @jpwhite3.

Lot's of good stuff here.

I agree - dropping Python 2 might be a good idea. Though, I think Python 2 is still default in Debian, so will need to look at how that affects things.

Before I can accept these changes though - please use the current pylint configuration in master as a base for this pull request. The current code base is using tabs for indentation rather than spaces for example and I'd like to keep it that way.

@jpwhite3
Copy link
Author

@adam-waldenberg - Sounds good.

I've updated the format recipe in the makefile to convert space indentation back to tabs. Running pipenv run make format will now use tabs by default.

I also added pylint to the series of commands run within the lint recipe. Running pipenv run make lint will now run a pylint check before running flake8.

As for the default python on Debian, I believe python3 has been available (but not default) for years and can be installed with apt install python3. The same is true for Ubuntu and other popular Debian derivatives. I'd be happy to help update documentation to account for this change.

@adam-waldenberg
Copy link
Member

Thank you @jpwhite3, this is looking better.

Yes, you are right about Python3 in Debian. It has been available for a long time, but I think version 2 is still the default version. But it should not be a big issue, as we can reference Python3 directly. As soon as we mane a new release with the switch I'm sure the Debian package maintainer can handle it as well.

Will look closer at the pull request as soon as I have time.

@wildone
Copy link

wildone commented Jul 31, 2021

+1 it's 2021 running this in a container so no need to worry about old-school stuff.

@ckastner
Copy link

@adam-waldenberg @jpwhite3 as the maintainer of the package in the official Debian archive, I'd wholeheartedly support a switch to Python3. As of Debian 11, Python2 is generally no longer installed on systems -- if I remember correctly, it's being kept around only for a handful of edge cases that still require it.

If this is just an issue concerning the #!/usr/bin/python shebang, don't worry about it -- we adjust those during package build.

@ckastner
Copy link

I forgot to mention the most important thing: the Debian version has already been ported to Python3; you can find the patch here.

I believe this was done using 2to3. The contributor did not forward this patch so I was about to, but then I found this PR.

@jpwhite3
Copy link
Author

jpwhite3 commented Mar 1, 2022

I also used 2to3 for the initial conversion, but also added the basic characterization tests to validate as much functionality as I could. In doing so I found issues with the conversion that ai had to fix by hand - so I am glad I did both. Since this PR was raised, I've moved a head slightly and dropped support (CI testing really) for Python 3.6 (as it is also end-of-life) and added support for Python 3.10.

@jpwhite3 jpwhite3 changed the title Drop support for Python 3.5 and lower Drop support for Python 3.6 and lower Mar 1, 2022
@adam-waldenberg
Copy link
Member

Thanks @jpwhite3. I will also go over it before I merge anything - if you find any other issues, make sure you update this PR. :)

@adam-waldenberg
Copy link
Member

adam-waldenberg commented May 4, 2022

@ckastner

@adam-waldenberg @jpwhite3 as the maintainer of the package in the official Debian archive, I'd wholeheartedly support a switch to Python3. As of Debian 11, Python2 is generally no longer installed on systems -- if I remember correctly, it's being kept around only for a handful of edge cases that still require it.

If this is just an issue concerning the #!/usr/bin/python shebang, don't worry about it -- we adjust those during package build.

I guess there's no stopping it now then. It's high time for a stable 0.5.0 release. This switch should be part of it.

@vesk4000
Copy link

Thanks a lot for this PR. It's very useful because I don't have Python 2 and it was unreasonably difficult to deal with that, so I just used your PR and it worked perfectly. It's only a shame it hasn't been merged yet.

@fuhrmanator
Copy link

Yes, waiting patiently for this merge (and eventually the updated npm for ease of install). 🚀

@seperman
Copy link

Seems like the maintainer has given up on gitinspector. @adam-waldenberg

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.

7 participants