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

[RFC] Remove Python 2 compatibility #1660

Closed
wants to merge 14 commits into from
Closed

[RFC] Remove Python 2 compatibility #1660

wants to merge 14 commits into from

Conversation

thiagokokada
Copy link
Contributor

@thiagokokada thiagokokada commented Jan 18, 2019

This is mostly a PR to RFC a removal of Python 2 compatibility, as discussed in issue #1584.

What I basically did:

  • Run 2to3 tool to help detect places where useless compatibility code was being used: u"", imports with old name, __future__ imports, etc.
  • Grep the code for "python 2", "python2", "py2", "python3", "python 3" and "py3", remove obvious code that was Python 2 specific.

Probably this should be missing things and also may introduce new bugs. However this PR serves as a benchmark on how much code can be simplified.

@lasers lasers added this to the 4.0 milestone Jan 18, 2019
@thiagokokada
Copy link
Contributor Author

thiagokokada commented Jan 18, 2019

Some of our modules use MySQLdb. MySQLdb never had a Python 3 release, so those modules wouldn't work in a Python 3 world only.

The modules:

  • py3status/modules/rt.py
  • py3status/modules/sql.py
  • py3status/modules/glpi.py

The only one of those that offers an alternative that works in Python 3 is py3status/modules/rt.py, that can also works with pymysql. pymysql seems to offer a compatible API, so maybe we could drop support to MySQLdb and substitute the usage of MySQLdb to pymysql. Any opnions?

@thiagokokada
Copy link
Contributor Author

Well, I think that is it for now. I am using this branch in my desktop to look for any regressions, however as far I tested for my configuration it is working very well.

I don't really know what you @ultrabug (and the other maintainers) will want to do with this PR. It was most for experiment, however it can be merged without problems.

@ultrabug
Copy link
Owner

Thanks for this amazing work @m45t3r !

So far, 4.0 dev has not started so we will not be dropping python2 support until we work on the 4 branch.

Your preliminary work will help a lot tho and could very well be the start of the 4.0 branch when we feel ready to move on this!

@cereal2nd
Copy link
Contributor

50218cf breaks the people that already use py3status with mysqldb, they will need to change there config

@thiagokokada
Copy link
Contributor Author

@cereal2nd if this PR is merged it would break anyway since AFAIK there is no port for MySQLdb for Python 3.

Unless I am wrong.

@cereal2nd
Copy link
Contributor

I'm talking about the sql module,
I applied the PR locally and everything works, except we place the 'MySQLdb' in the config, and we have to replace this with 'pymysql', so this will be a backward incompatibility

@thiagokokada
Copy link
Contributor Author

@cereal2nd Huh, yeah, I understand that, however this incompatibility is unavoidable.

Probably we should document it somewhere (in release notes or something).

@thiagokokada
Copy link
Contributor Author

Rebased and add a preliminary changelog.

@ultrabug
Copy link
Owner

ultrabug commented Apr 3, 2020

@thiagokokada hi mate

as you may have noticed, we have worked on removing support for PY2 now thx to @hugovk

does that sound okay to close this PR so you can maybe add your own new ideas on it if you wish to?

@thiagokokada thiagokokada deleted the python3-only branch April 3, 2020 19:15
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.

4 participants