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

STY: Apply pyupgrade suggestions #3147

Closed

Conversation

DimitriPapadopoulos
Copy link
Contributor

Changes proposed in this pull request

pyupgrade --py310-plus

Also manually change a few .format() calls to f-strings.

Copy link

codecov bot commented Nov 21, 2023

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (0e9fc42) 50.00% compared to head (e09e36a) 50.00%.

Files Patch % Lines
fmriprep/workflows/base.py 0.00% 4 Missing ⚠️
fmriprep/interfaces/reports.py 0.00% 3 Missing ⚠️
fmriprep/interfaces/workbench.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3147   +/-   ##
=======================================
  Coverage   50.00%   50.00%           
=======================================
  Files          53       53           
  Lines        3914     3916    +2     
=======================================
+ Hits         1957     1958    +1     
- Misses       1957     1958    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@effigies
Copy link
Member

I want to hold off on this until the full release, then we'll probably just do what we're doing over in smriprep. We can take care of pyupgrade, flake8, and all that in a single PR.

pyupgrade --py310-plus

Also manually change a few `.format()` calls to f-strings.
effigies added a commit that referenced this pull request Feb 5, 2024
Follow-up to nipreps/smriprep#397. This should
replace #3185 and #3147.

I would review this commit-by-commit, and skip
c0f2183, which was a pretty safe run of
`ruff format` and `ruff --fix`. It's dominated by quote changes, so I
would ignore it. Everything else could stand a review.
@effigies
Copy link
Member

I think this should be covered by ruff now.

@effigies effigies closed this Feb 29, 2024
@DimitriPapadopoulos
Copy link
Contributor Author

Not entirely. For example, switching to f-strings is not covered by ruff (pyupgrade rules).

@effigies
Copy link
Member

Ah, do you want to go ahead and re-upgrade? Note that wrapper/ needs to be excluded, since we're still maintaining Python 2.7 compatibility for that package.

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Feb 29, 2024

Let me have a look first:

  • exclude wrapper
  • perhaps add other ruff rules that may be associated to the pyupgrade suggestions

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