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

python: update required versions of black and mypy #5808

Merged
merged 2 commits into from Mar 20, 2024

Conversation

grondo
Copy link
Contributor

@grondo grondo commented Mar 20, 2024

Dependabot noted a potential security issue in our currently required version of black in #5806, but a bit more than just bumping the version was required to get the suggested black version working, so I've done that here. This included apply the very minor formatting changes enacted by the new version.

The changes also required an update of mypy, so I've bumped that to the latest version as well. There did not seem to be any fallout.

Pending PRs may have to reformatted if they include python code. However, I was surprised how little changed in the formatting when updating to the new black version.

Problem: Github dependabot notified this project of a security
issue with black 22.8.0 and recommends updating to 24.3.0.

Update black requirement to 24.3.0 and commit all formatting changes
as a result.

Also make the following other required changes to get this version
of black to work:

Ensure the python linting action runs on ubuntu-latest in github,
otherwise this version of black is not found by pip.

Update pyproject.toml to remove tool.black.profile, which the newer
black doesn't understand or support.

Update pyproject.toml to remove tool.black.exclude, which wasn't
being used under pre-commit anyway. Replace with force-exclude,
which applies the exclusion pattern even if the targets are passed
as a list of files as is done with pre-commit.
Problem: The version of mypy pinned in scripts/requirements-dev.txt
is too old for the github ubuntu-latest runner.

Update to the latest mypy version 1.9.0.
Copy link
Member

@garlick garlick left a comment

Choose a reason for hiding this comment

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

Thanks for handling this. LGTM!

@grondo
Copy link
Contributor Author

grondo commented Mar 20, 2024

Thanks, I've set MWP.

Copy link

codecov bot commented Mar 20, 2024

Codecov Report

Merging #5808 (617055a) into master (eaaf293) will increase coverage by 28.49%.
The diff coverage is 100.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #5808       +/-   ##
===========================================
+ Coverage   54.85%   83.34%   +28.49%     
===========================================
  Files         461      509       +48     
  Lines       74783    82485     +7702     
===========================================
+ Hits        41020    68749    +27729     
+ Misses      33763    13736    -20027     
Files Coverage Δ
src/bindings/python/flux/cli/base.py 95.62% <ø> (ø)
src/bindings/python/flux/job/info.py 93.76% <100.00%> (+71.98%) ⬆️
src/bindings/python/flux/util.py 95.65% <100.00%> (+70.65%) ⬆️
src/bindings/python/flux/wrapper.py 96.68% <ø> (+18.39%) ⬆️
src/cmd/flux-pgrep.py 95.12% <ø> (ø)

... and 427 files with indirect coverage changes

@mergify mergify bot merged commit 51d7bb1 into flux-framework:master Mar 20, 2024
35 checks passed
@grondo grondo deleted the black-update branch March 20, 2024 21:53
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.

None yet

2 participants