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

Remove Python 2 compatibility and use of six module #2688

Merged
merged 8 commits into from
Feb 4, 2025

Conversation

swt2c
Copy link
Collaborator

@swt2c swt2c commented Feb 3, 2025

  • Drop support for building with Python 2. wxPython Phoenix’s build process and build scripts used to generate the code now only use the project’s supported Python 3 versions.
  • Drop Python 2 compatibility in samples and demos. Using six is not the recommended pattern when using wxPython Phoenix
  • ListCtrl.Append() uses ˋstr` for setting and inserting items
  • ˋwxPixelDataBaseˋ’s iterator works only for Python 3.
  • ˋwx.DateTimeˋ’s string representation drops support for Python 2 UTF8 output.
  • Sphinx docs generation and tweaks work with Python 3 only. Docs are not generally generated by wxPython’s users.
  • Unit tests for changes listed above have been adapted accordingly
  • wx.lib.agw.aui, wx.lib.agw, wx.lib.fancytext, wx.lib.floatcanvas, wx.lib.masked.maskededit, wx.lib.softwareupdate, wx.py.filling, wx.py.shell, wx.tools.wxget, and more drops support for Python 2.

@echoix
Copy link
Contributor

echoix commented Feb 3, 2025

Ruff has multiple rules coming from different linters and plugins for linters that can also help with this kind of task. Once a pyproject.toml is present, and we configure for what minimum Python version we need to be compatible, it can help us further modernize that code.

@swt2c swt2c force-pushed the remove_python2_six branch from 7fa191f to a29322f Compare February 4, 2025 01:32
@swt2c swt2c merged commit da22777 into wxWidgets:master Feb 4, 2025
13 checks passed
@swt2c swt2c deleted the remove_python2_six branch February 4, 2025 02:50
@Metallicow
Copy link
Contributor

Metallicow commented Feb 5, 2025

BOOKMARKING: I knew this day would come eventually, but I expected as Robin stated it would break...
In this case, it isn't broke yet..., because it was a manual decision.

I'm sure there are still a few out there that maintain a xwx py2/3 cross compatible version of some modules(NOTE: agw/aui).
I think along the course of all this, most things could have been written uglier such as the iteritems sections, but believe it or not the first person that forced me to copy/paste/maintain any other module was with the refactoring of agw.thumbnailctrl Haha.

I can imagine what these folks will think when they see six has been removed. and possibly have to include that single file, let alone write a patcher for this commit.

I use a single file wrapper for most all widgets, so probably not much change there, as CLASSIC is Dead and I've since removed support for it, but Python2 and phoenix wxPython 4.0.7.post2 is still fastest for me. As for Python3 and anything after wxPython 4.0.7.post2, things could still be a lot better in certain areas.

Kinda funny I got to write such a short history from Py2.7-3.13

@Metallicow
Copy link
Contributor

Metallicow commented Feb 5, 2025

from .. import (policies) This might create an error.... No trailing comma or etc...

from importlib.util import cache_from_source User might need to backport importlib or keep a local copy

from six.moves import builtins This means if need be, to include overridding this in your startup code...

argspec = inspect.getfullargspec(obj) This is a namespace change. Dynamic Coder probably mixed up with your code or the interactive interpreter. Simple if else hasattr statement.

for t in tokenize.tokenize(f.readline): If namespace or code changes here, expect a HARDCRASH

output = ' %s' % part Maybe padding this would be more readable...

@Metallicow
Copy link
Contributor

Metallicow commented Feb 5, 2025

That is a first review of the commit(reading it manually). Haven't run it yet. May be some stragglers typos or other such...
@everyoneinvolved Thanks for attempting this convoluted issue.
@swt2c
@echoix
Please review this commit. Above comments, etc...

@echoix
Copy link
Contributor

echoix commented Feb 5, 2025

What kind of review would you like? It's already merged though.

@Metallicow
Copy link
Contributor

What kind of review would you like? It's already merged though.

@echoix Considering you participated fully in the evolution/commits of "Project Phoenix" and it was to span Py2/3("as per Robin Dunn") and work without errors(may we will call them execptions), It would be accepting if you reviewed your knowledge contained in this commit as to how to revert it if your 'sayso's' might contain any errors and exceptions. There should be None.

Basically my statement means you should have run it on Python 2.7.18 -Python 3.13 and reported back the differences(and your knowledge of why we changed anything but a one-liner(string-replace)). This is highly valuable information.

... You never know how many people will drop me a $1000 considering nothing has changed in over 10 years in the code. (BTW they do not pay me anything!)

I'd just like you to run it on Python 2.7.18 , and document the changes that will happen throughly. I understand this isn't your job to do this, but would make your review completely backwards-compatible.

Thanks.

@echoix
Copy link
Contributor

echoix commented Feb 5, 2025

@Metallicow note that following #2664, where the policy for what Python versions to support (including syntax-wize), #2689 added this information to the pyproject.toml, with

requires-python = ">= 3.9"

This indicates to tools that they are free to use features and syntax that Python 3.9 and newer supports. Newer features not available for 3.9 won't be suggested yet. I think pip and build backends take this constraint into account too.

@Metallicow
Copy link
Contributor

I am simply asking beforehand that folks that step through this will have issues. Please document specifically the revert change from Py3/2 into Py3(solo) that are being performed here.

Lets say I NEVER helped do CLASSIC->PHOENIX. How would you deal with PHOENIXPY23->PHOENIX3?

???

@echoix
Copy link
Contributor

echoix commented Feb 5, 2025

Lets say I NEVER helped do CLASSIC->PHOENIX. How would you deal with PHOENIXPY23->PHOENIX3?

Oh you mean like a migration guide but for Python 2 to 3, but for wxPython specifically?

@Metallicow
Copy link
Contributor

Metallicow commented Feb 5, 2025

@echoix Note No description provided.
Yes, basically Py2/3 to Py3

@echoix
Copy link
Contributor

echoix commented Feb 5, 2025

I'm starting to write a little bullet point list in the form of what could be found in a changelog/release notes. It could be copied to the PRs description by someone with write permissions or more.

@Metallicow
Copy link
Contributor

NOTE: that if it is only PY3 then it is not "Project Phoenix" anymore...
@swt2c @echoix How would you deal with this, considering the repo is "called"() as "phoenix"
???

@echoix
Copy link
Contributor

echoix commented Feb 5, 2025

I don't see the link of "project Phoenix" with indefinite Python 2 and Python 3 compatibility guarantees.
I'm quite new to wxPython Phoenix, I only actively know about it since 2 years, when I actively started maintaining the GRASS GIS project that heavily uses wxPython. Only recently I started taking a look at the repo and how wxPython works.

In the GRASS GIS project, that is generally on the conservative side and keeps a lot of compatibility, we recently decided to stop supporting the dual wxPython api (classic and 4+ aka Phoenix). We didn't who needed the latest and greatest GRASS (with the other compatible versions of dependencies) that only had access to such an old wxPython (classic version). We didn't remove it yet everywhere, but when files are changed the decision is already made to remove the dual code paths if needed.

So, what I understood that project Phoenix meant, was that it was the reborn version of wxPython (Classic) wxWidgets Python bindings, with a different and newer bindings generator, that was more modern at that time. My impression from seeing a bit of the history this summer, is that there was a kind of slowdown in improvements and innovations in the last 3+ years. It's understandable, but I don't think this should mean a statu quo on the supported versions, which I think should be continuously following the contemporary versions of the tools.

@echoix
Copy link
Contributor

echoix commented Feb 5, 2025

I dug out the first post that announced the wxPython Phoenix, in August 2016. https://wxpython.org/news/phoenix-on-the-way/index.html
Lots have changed in packaging and tooling for Python, including the approach of compatibility since.

I remember in summer 2018, writing Python 2 and 3 compatible code was already starting to be outdated, and the Python 2 drop fear was partly gone already.
Lots and lots of projects have dropped the Python 2 compatibility in the recent years.

Python 3.9 marks the time where Python removed or would remove the compatibility layers and depreciation warnings that were emitted for years. Now that Python 3.9 is the oldest non-EOL Python version, dropped support for Python 2 shouldn't come to a surprise for anyone, and if so, wxPython Phoenix wouldn't be their first problem, as they would have had problems with about any other project they use too.
https://docs.python.org/3/whatsnew/3.9.html#you-should-check-for-deprecationwarning-in-your-code

@echoix
Copy link
Contributor

echoix commented Feb 5, 2025

What about not taking too much energy for hypothetical situations where not having Python 2 would be problematic and a surprise, writing a little bit quickly but not too much, and help out on a case by case basis if ever some help is needed?

@echoix
Copy link
Contributor

echoix commented Feb 6, 2025

So I reread the changed files, there's not much that caught my eyes for the parts where the Python files inside the wx folder. The files before, where the generator runs on for example, might have more impact.
Users should not worry about Sphinx docs generation, that more of a concern for the maintainers here.

So, here's a little summary, my interpretation of this PR:
Changelog:

  • Drop support for building with Python 2. wxPython Phoenix’s build process and build scripts used to generate the code now only use the project’s supported Python 3 versions.
  • Drop Python 2 compatibility in samples and demos. Using six is not the recommended pattern when using wxPython Phoenix
  • ListCtrl.Append() uses ˋstr` for setting and inserting items
  • ˋwxPixelDataBaseˋ’s iterator works only for Python 3.
  • ˋwx.DateTimeˋ’s string representation drops support for Python 2 UTF8 output.
  • Sphinx docs generation and tweaks work with Python 3 only. Docs are not generally generated by wxPython’s users.
  • Unit tests for changes listed above have been adapted accordingly
  • wx.lib.agw.aui, wx.lib.agw, wx.lib.fancytext, wx.lib.floatcanvas, wx.lib.masked.maskededit, wx.lib.softwareupdate, wx.py.filling, wx.py.shell, wx.tools.wxget, and more drops support for Python 2.

@Metallicow
Copy link
Contributor

@echoix I think the biggest changes are the fact that this commit broke code(s) without specifiying a deprecation immediatedly beforehand.
@swt2c Having a immediate release upon base prior would be standard, not inbetween releases. I hope you could work this out if it is the issue.

Needless to say, there is a shitton of changes inbetween point A and point B. We was all a part of making all those changes.

Personally I'd like a version built for EVERY Python on 'Robins departure' Version

@RobinD42 Should decide this. This is his baby. He is not dead yet. There is a community afterwards. 2-10 folks shouldn't make such a big decision immediately for all of us. I know you hate straddling code, but it was created for a reason. That reason is "Project Phoenix"

@swt2c
Copy link
Collaborator Author

swt2c commented Feb 6, 2025

@Metallicow I didn't have a description because this was a replacement for a previous PR ( #2540) that had been open for nearly a year with no negative feedback.

In reality, Python 2 support has been slowly bit rotting for a while. It is likely that you could not build Phoenix for Python 2 for quite some time. SIP certainly does not support Python 2 anymore. In fact, the last time Robin built wheels for Python 2 was for 4.0.7.post2 in 2019.

@Metallicow
Copy link
Contributor

Ok. So if you reverted this Commit tomorrow and buildt the "Farewell Robin" RELz there would be no problem right. Well.

There should not be "Breaking releases", a "breaking commit might be called a nightly"

@echoix

So I reread the changed files, there's not much that caught my eyes for the parts where the Python files inside the wx folder. The files before, where the generator runs on for example, might have more impact. Users should not worry about Sphinx docs generation, that more of a concern for the maintainers here.

So, here's a little summary, my interpretation of this PR: Changelog:

* Drop support for building with Python 2. wxPython Phoenix’s build process and build scripts used to generate the code now only use the project’s supported Python 3 versions.

* Drop Python 2 compatibility in samples and demos. Using six is not the recommended pattern when using wxPython Phoenix

* ListCtrl.Append() uses ˋstr` for setting and inserting items

* ˋwxPixelDataBaseˋ’s iterator works only for Python 3.

* ˋwx.DateTimeˋ’s string representation drops support for Python 2 UTF8 output.

* Sphinx docs generation and tweaks work with Python 3 only. Docs are not generally generated by wxPython’s users.

* Unit tests for changes listed above have been adapted accordingly

* wx.lib.agw.aui, wx.lib.agw, wx.lib.fancytext, wx.lib.floatcanvas, wx.lib.masked.maskededit, wx.lib.softwareupdate, wx.py.filling, wx.py.shell, wx.tools.wxget, and more drops support for Python 2.
  • I am OK with dropping Python2 support considering there is a full fledged backup! AND it is EXPLICITLY noted beforehand.
  • As far as the demos are concerned, I or others can uphold backwards compatibility. DO NOT worry about the demo.py files.
  • ListCtrl changes might be a concern for some, but not to my code (wxPython) editor.
  • If DateTime is fucked up DateTime will be fucked up.

What about not taking too much energy for hypothetical situations where not having Python 2 would be problematic and a surprise, writing a little bit quickly but not too much, and help out on a case by case basis if ever some help is needed?

WHY? Cause a surprise, often means, you just suggested to everyone, you live in a different world. Eat Missile. NoWayBackMachine.

  • Docs changes are a normal thing, shouldn't have to woory too much about backporting typos.
  • Baaaa. Unit Tests are for real soldiers. Run that code.
  • wx.lib.agw, and, wx.py WILL be of interest... WHY Because they take on the job of becoming GOD.
    You don't tell me what to do...

@swt2c There should at the very least be a "Farewell Robin" RELz. If you have to revert this commit, build it and RELz it, then do the opposite, git doesn't give a shit. Tho remind yourself, your users do...

If I can walk from python2.7.18 to 3.13+ then why are you torturing individuals?

@echoix
Copy link
Contributor

echoix commented Feb 6, 2025

No need to revert anything. This is git, so go back to the commit you want, make a branch, and prepare what you think should be a farewell robin tag. It doesn't need to be released as is, but could be tagged as a patch version in between the current releases.

@Metallicow
Copy link
Contributor

Metallicow commented Feb 6, 2025

@echoix We don't rely on git, we rely on user supporting the project. Do you have a hosting server by chance? or is your name torvalds?

EDIT: We manually review every commit we can. If you send a '.txt' file via email, we still have to review 2 '.txt' files. Thanks.

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