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

Convert nested if statements to elif clauses for better readability. #10227

Closed

Conversation

ananyakaligal
Copy link
Contributor

Closes #10196

PLR5501 - Convert nested if statements to elif clauses for better readability.

@RayBB I converted nested if statements to elif clauses for better readability across 14 files.
In some cases, I couldn't apply the change due to specific conditions, so I simplified or improved readability instead.
I’ve reviewed all the changes carefully, but if I’ve missed any files, please let me know. Let me know if further modifications are needed.

@RayBB
Copy link
Collaborator

RayBB commented Dec 29, 2024

Please tag me once all tests and precommit checks are passing.

@ananyakaligal ananyakaligal marked this pull request as draft December 29, 2024 22:39
@ananyakaligal
Copy link
Contributor Author

@RayBB All the checks have passed. Let me know if any more modifications are needed.

Copy link
Collaborator

@RayBB RayBB left a comment

Choose a reason for hiding this comment

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

Please remove any changes that aren't strictly needed to enable the rule. For the most part you should only have to enable the rule and let ruff fix thing. There may be a few other small changes needed. Don't add other changes like spacing unless important.

@ananyakaligal
Copy link
Contributor Author

@RayBB Thank you for the feedback, I will work on it and make a PR by tomorrow.
In some cases, I may not be able to completely remove the nested conditions, but I can work on reducing their complexity or nesting level. Would that be acceptable?

@RayBB
Copy link
Collaborator

RayBB commented Dec 30, 2024

We should let ruff fix them automatically. If any are too complicated for ruff to fix let's review together on a case by case basis.

@ananyakaligal
Copy link
Contributor Author

@RayBB alright, I'll revert the all the unnecessary changes made and review each case properly this time. Thankyou

@ananyakaligal
Copy link
Contributor Author

Hello @RayBB , Due to unforeseen circumstances, I couldn’t work on this as planned. I’ll ensure it’s completed in the next 2-3 days.

@RayBB
Copy link
Collaborator

RayBB commented Jan 3, 2025

@ananyakaligal are you still working on this?

@ananyakaligal
Copy link
Contributor Author

@RayBB I just made a PR on this #10260

@ananyakaligal ananyakaligal deleted the issue10196-PLR5501 branch January 3, 2025 11:43
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.

Re-enable several disabled ruff rules
2 participants