Skip to content

PYTHON-2504 Apply one time pyupgrade --py37-plus v3.3.2 and ruff #1196

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
May 11, 2023
Merged

PYTHON-2504 Apply one time pyupgrade --py37-plus v3.3.2 and ruff #1196

merged 3 commits into from
May 11, 2023

Conversation

ShaneHarvey
Copy link
Member

@ShaneHarvey ShaneHarvey commented Apr 26, 2023 •
edited
Loading

https://jira.mongodb.org/browse/PYTHON-2504

Successor to #882.

Ran this repeatedly until it stablized after 3 or 4 runs:

$ pyupgrade --py37-plus bson/*.py pymongo/*.py gridfs/*.py test/*.py tools/*.py test/*/*.py

I also ran ruff 0.0.265 like this (see https://beta.ruff.rs/docs/rules/):

$ ruff --fix-only --select ALL --fixable ALL --target-version py37 --line-length=100 --unfixable COM812,D400,D415,ERA001,RUF100,SIM108,D211,D212,SIM105,SIM,PT,ANN204,EM bson/*.py pymongo/*.py gridfs/*.py test/*.py test/*/*.py

The --unfixable codes suppress some unneeded or particularly noisy changes.

As noted in the previous PR:

I tested out adding this to pre-commit but pyupgrade's formatting conflicts with black so it's more of a nuisance.

I'll add this to .git-blame-ignore-revs after the PR is merged.

@ShaneHarvey
Copy link
Member Author

I'm waiting until we get mypy working to mark this as ready.

@ShaneHarvey ShaneHarvey mentioned this pull request Apr 28, 2023
Merged
@ShaneHarvey ShaneHarvey marked this pull request as ready for review April 28, 2023 21:43
@ShaneHarvey ShaneHarvey requested a review from blink1073 as a code owner April 28, 2023 21:43
@ShaneHarvey ShaneHarvey changed the title PYTHON-2504 Apply pyupgrade --py37-plus v3.3.2 PYTHON-2504 Apply one time pyupgrade --py37-plus v3.3.2 and ruff Apr 28, 2023
@blink1073
Copy link
Member

The mypy --strict is failing, not sure why it is being so strict. Perhaps we need to tell it to ignore imports for pymongo and bson and focus just on the code in the test?

@ShaneHarvey
Copy link
Member Author

I'm going to back out the mypy changes for this PR and open a new ticket.

@ShaneHarvey
Copy link
Member Author

I reopened https://jira.mongodb.org/browse/PYTHON-3679 to track mypy --strict support.

blink1073
blink1073 approved these changes May 1, 2023
View reviewed changes
Copy link
Member

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

LGTM!

@ShaneHarvey
Copy link
Member Author

I'm going to wait until #1138 is merged to avoid adding merge conflicts to that PR.

@ShaneHarvey
Copy link
Member Author

Rebased and reran the upgrade commands. Waiting for tests to pass.

ShaneHarvey
ShaneHarvey commented May 11, 2023
View reviewed changes
bson/__init__.py Outdated
@@ -269,10 +267,12 @@ def _get_string(
length = _UNPACK_INT_FROM(data, position)[0]
position += 4
if length < 1 or obj_end - position < length:
raise InvalidBSON("invalid string length")
msg = "invalid string length"
raise InvalidBSON(msg)
Copy link
Member Author

Choose a reason for hiding this comment

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

Ew I don't like what ruff is doing here at all. Looking for a code to disable.

Copy link
Member

Choose a reason for hiding this comment

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

That rule comes from https://pypi.org/project/flake8-errmsg/

Copy link
Member

Choose a reason for hiding this comment

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

https://beta.ruff.rs/docs/rules/#flake8-errmsg-em

Copy link
Member Author

Choose a reason for hiding this comment

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

Caused by https://beta.ruff.rs/docs/rules/#flake8-errmsg-em

Disabling now.

Copy link
Member Author

Choose a reason for hiding this comment

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

FWIW I do actually agree that the exception traceback is clearer with the temp variable but I just don't like it from a code readability standpoint.

@ShaneHarvey
ruff --fix-only --select ALL --fixable ALL --target-version py37 --li…
713d0a9 
…ne-length=100 --unfixable COM812,D400,D415,ERA001,RUF100,SIM108,D211,D212,SIM105,SIM,PT,ANN204,EM bson/*.py pymongo/*.py gridfs/*.py test/*.py test/*/*.py
@ShaneHarvey
PYTHON-2504 Fix CodecOptions __repr__
082b429
@ShaneHarvey ShaneHarvey requested a review from blink1073 May 11, 2023 22:05
blink1073
blink1073 approved these changes May 11, 2023
View reviewed changes
Copy link
Member

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

I did some spot checks, LGTM!

@ShaneHarvey ShaneHarvey merged commit 0092b0a into mongodb:master May 11, 2023
@ShaneHarvey ShaneHarvey deleted the PYTHON-2504 branch May 11, 2023 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blink1073 blink1073 blink1073 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ShaneHarvey @blink1073