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 Python2 hybridation code #2540

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

Conversation

a-detiste
Copy link

print() is available since Python 2.6

https://wiki.debian.org/Python3-six-removal

@@ -1,5 +1,6 @@
from __future__ import print_function
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably don't need this...we can drop support for Python 2.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so let's remove all sixusage ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and the vendored copy living as wx/lib/pubsub/py2and3.py

@a-detiste a-detiste changed the title remove usage of six.print_() remove Python2 hybridation code Mar 21, 2024
@@ -550,7 +549,7 @@ def __init__(self, parent):
IL_VARIABLE_SIZE = 1

# Python integers, to make long types to work with CreateListItem
INTEGER_TYPES = six.integer_types
INTEGER_TYPES = int

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is changing the value from a list to a scalar value which is now causing crashes later on in the source. I converted all in INTEGER_TYPES to == INTEGER_TYPES and it fixed it. Should probably update the name to reflect the change as well.

if type(itemOrId) in INTEGER_TYPES: < line 598

Another example:

    """
    A list event holds information about events associated with :class:`UltimateListCtrl`
    objects.
    """

    def __init__(self, commandTypeOrEvent=None, winid=0):
        """
        Default class constructor.
        For internal use: do not call it in your code!

        :param `commandTypeOrEvent`: the event type or another instance of
         :class:`PyCommandEvent`;
        :param `winid`: the event identifier.
        """

        if type(commandTypeOrEvent) in INTEGER_TYPES:

@a-detiste
Copy link
Author

a-detiste commented Mar 29, 2024 via email

@jmoraleda
Copy link
Contributor

I agree with this PR. I also agree with removing all six usage.

@komoto48g
Copy link
Contributor

I agree with dropping support for Python 2.
IMHO, this PR is too aggressive and the first commit (removing print_) would be more acceptable if it was a separate PR.

From "https://wxpython.org/pages/contributor-guide/",
5. Do not put more than one fix/feature in the same branch or PR. Make a new distinct branch and PR for each of them.

@a-detiste
Copy link
Author

I m on Holliday. Anyone can cherrypick the first commit that removes py2.6< compatibility if it tastes them,
I ll redo this work in smaller pieces when I get back.

@a-detiste
Copy link
Author

At first there was only this one first commit untill I got asked to proceed further

@newville
Copy link
Contributor

newville commented Aug 5, 2024

Please merge.

@a-detiste
Copy link
Author

The PR against GRASS (which uses Wx by the way) had a similar +/- size and went in smoothly. I m not scared. What do we need to go further ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants