ejwa / gitinspector

:bar_chart: The statistical analysis tool for git repositories
GNU General Public License v3.0
2.34k stars 324 forks source link

Drop support for Python 3.6 and lower #223

Open jpwhite3 opened 3 years ago

jpwhite3 commented 3 years ago

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:

adam-waldenberg commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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 commented 2 years ago

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

ckastner commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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.

adam-waldenberg commented 2 years ago

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 commented 2 years ago

@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 commented 1 year ago

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 commented 1 year ago

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

seperman commented 2 months ago

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