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 support for Python 2 and require > 3.6 #114

Merged
merged 11 commits into from
Nov 30, 2021
Merged

Conversation

Bomme
Copy link
Contributor

@Bomme Bomme commented Nov 30, 2021

I used pyupgrade to do an automatic upgrade. Then looked through the modules to remove code that explicitly mentioned Python 2.

I don't have any experience with travis, so I guessed how the new configuration could look like :)

Closes #112.

pyupgrade --py36-plus
t is recommended that you:

    Remove 'pytest-runner' from your setup_requires, preferably removing the setup_requires option.
    Remove 'pytest' and any other testing requirements from tests_require, preferably removing the tests_requires option.
    Select a tool to bootstrap and then run tests such as tox.
Copy link
Member

@sampsyo sampsyo left a comment

Choose a reason for hiding this comment

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

This all looks good to me! And I don't, on first glance, see any other Python 2-isms remaining.

In the absence of CI for PRs, can I assume that tox revealed the tests are still passing? (It would be great to set up GitHub Actions here someday…)

It occurs to me that we should probably bump the version (not sure to what—is 2.2.0 or 3.0.0 the correct SemVer increment?) and add a quick chanelog entry to the README. Then I think it makes sense to ship this sooner rather than later

@Bomme
Copy link
Contributor Author

Bomme commented Nov 30, 2021

I found two things that I did not touch yet which are probably not required anymore in py3:

            # In Python 3, result of reading from stderr is bytes.
            if isinstance(line, bytes):
                line = line.decode('utf8', 'ignore')

and

# Python 3.4 added support for 24-bit (3-byte) samples.
SUPPORTED_WIDTHS = (1, 2, 3, 4)

But I didn't want to change too many things at once.

@Bomme
Copy link
Contributor Author

Bomme commented Nov 30, 2021

I replaced travis with GitHub Actions and tests are passing. (I guess the Actions appear in this repo after this is merged. For now you can check them in my fork)

I guess a new major version would be the correct way. This gives the possibility of including other breaking changes and only do one release (if you plan to do any other breaking changes)

@sampsyo
Copy link
Member

sampsyo commented Nov 30, 2021

Nice!! Yes, it does seem just fine to leave some of that stuff to future PRs; picking the low-hanging fruit seems like the way to go.

And thank you for setting up GitHub Actions!!! This is super great! I'll merge this now and bump the version.

@sampsyo sampsyo merged commit b33dc1e into beetbox:master Nov 30, 2021
sampsyo added a commit that referenced this pull request Nov 30, 2021
As discussed in #114.
@Bomme Bomme deleted the py36 branch December 3, 2021 13:17
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.

Drop Python 2 support?
2 participants