Skip to content

Fix invalid f-strings #7457

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 12 commits into from
Aug 16, 2025
Merged

Fix invalid f-strings #7457

merged 12 commits into from
Aug 16, 2025

Conversation

cyyever
Copy link
Contributor

@cyyever cyyever commented Jul 29, 2025 •
edited
Loading

Fix invalid f-strings detected by ruff.

@loadams
Copy link
Collaborator

loadams commented Jul 29, 2025

@cyyever - could you please run the formatter? Also is it worth adding ruff to the formatter?

@cyyever
Use 'not in'
6c0f46b 
Signed-off-by: cyy <[email protected]>
@cyyever
Copy link
Contributor Author

cyyever commented Jul 30, 2025 •
edited
Loading

@loadams I have formatted the files. I think it is worth adding ruff. However, to reduce the impact of refactoring we can apply the changes gradually.

@cyyever cyyever changed the title Fix some ruff warnings Fix invalid f-strings Jul 31, 2025
@loadams loadams requested a review from sfc-gh-mwyatt July 31, 2025 15:58
@loadams
Copy link
Collaborator

loadams commented Jul 31, 2025

cc: @sfc-gh-mwyatt

@cyyever
Copy link
Contributor Author

cyyever commented Aug 10, 2025

The test failures are unrelated.

@sfc-gh-mwyatt
Copy link
Collaborator

FYI @loadams we probably don't want to add ruff to the formatting unless we have very good reason to. It could result in very big changes to the code base. Right now we are using yapf and flake8 to do the job that ruff would do. The reason this error is not caught by flake8 is because we have an exception for F541:

DeepSpeed/.flake8

Line 2 in 8aadf6c

ignore = E,F403,F405,F541,F841,W
. I would remove this before switching to a different linter/formatter.

@sfc-gh-truwase sfc-gh-truwase enabled auto-merge (squash) August 15, 2025 16:22
@sfc-gh-truwase sfc-gh-truwase merged commit 1c03d1b into deepspeedai:master Aug 16, 2025
15 of 17 checks passed
@cyyever cyyever deleted the ruff_fix branch August 18, 2025 02:29
LYMDLUT pushed a commit to LYMDLUT/DeepSpeed that referenced this pull request Aug 20, 2025
@cyyever @loadams
@sfc-gh-truwase @sfc-gh-mwyatt @LYM-fire
Fix invalid f-strings (deepspeedai#7457)
91bb16b 
Fix invalid f-strings detected by ruff.

---------

Signed-off-by: cyy <[email protected]>
Co-authored-by: Logan Adams <[email protected]>
Co-authored-by: Olatunji Ruwase <[email protected]>
Co-authored-by: Michael Wyatt <[email protected]>
Signed-off-by: lym <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sfc-gh-mwyatt sfc-gh-mwyatt sfc-gh-mwyatt approved these changes

@sfc-gh-truwase sfc-gh-truwase sfc-gh-truwase approved these changes

@tjruwase tjruwase Awaiting requested review from tjruwase tjruwase is a code owner

@tohtana tohtana Awaiting requested review from tohtana tohtana is a code owner

@jomayeri jomayeri Awaiting requested review from jomayeri jomayeri is a code owner

@loadams loadams Awaiting requested review from loadams loadams is a code owner

@hwchen2017 hwchen2017 Awaiting requested review from hwchen2017 hwchen2017 is a code owner

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

Successfully merging this pull request may close these issues.
4 participants
@cyyever @loadams @sfc-gh-mwyatt @sfc-gh-truwase