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 redundant six dependency #781

Merged
merged 6 commits into from
Jul 20, 2024
Merged

Conversation

hugovk
Copy link
Contributor

@hugovk hugovk commented Jul 20, 2024

Description

Describe what this change achieves.

This package only supports Python 3, however it still uses the six compatibility library to support Python 2 and 3.

This PR removes the dependency, and replaces the six calls with direct Python 3 code.

This makes the install a bit quicker, smaller, and the code a bit faster by using the standard library instead of a package.

Issues Resolved

List any issues this PR will resolve, e.g. Closes [...].

None known.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.


I did this using the https://github.com/asottile/pyupgrade tool that upgrades Python syntax.

We could run it on all the files, and upgrade to Python 3.8+ syntax (--py38-plus), but I wanted to keep the diff small, so I only ran it on files using six, and did the lowest syntax upgrade (--py3-plus):

rg six -l | xargs pyupgrade --py3-plus

(I do recommend running it on the whole codebase and for 3.8+, perhaps in another PR. For example pyupgrade **/*.py --py38-plus upgrades 84 further files.)


This PR also removes creation of universal wheels, that's only needed when supporting Python 2, and corrects the Black target version.

Signed-off-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Signed-off-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Signed-off-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Signed-off-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Signed-off-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Signed-off-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Looks good, thank you! Would definitely appreciate future upgrades.

@dblock dblock merged commit de96d28 into opensearch-project:main Jul 20, 2024
60 checks passed
@hugovk hugovk deleted the rm-six branch July 20, 2024 16:38
dblock pushed a commit to dblock/opensearch-py that referenced this pull request Aug 15, 2024
* Don't create universal wheel for Python 3 only

Signed-off-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>

* Update Black target version to match min Python supported

Signed-off-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>

* Upgrade files using six to Python 3 syntax

Signed-off-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>

* Remove redundant six dependency

Signed-off-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>

* Format with Black

Signed-off-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>

* Add changelog entry

Signed-off-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>

---------

Signed-off-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
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.

2 participants