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

Ruff: Disallow unsafe fixes and revert changes. #303

Merged
merged 4 commits into from
Aug 15, 2024

Conversation

ioannis-vm
Copy link
Contributor

Issue description

The ruff configuration in pyproject.toml contained preview=true, which enables certain checks (and potential fixes) that are very recent and potentially unstable. I applied some of those fixes in PR #297, instead of simply silencing the lines causing the warnings. This PR is the result of a review of those particular fixes.

Steps taken

  1. I identified all ruff rules currently "in preview" that have automated fixes from the ruff documentation.
  2. I checked out the original commit where I started applying fixes (9423954) and isolated all ruff warnings related to those specific rules.
  3. I removed duplicates, resulting in the following set of warnings:
+ E226 [*] Missing whitespace around arithmetic operator
+ FURB171 [*] Membership test against single-item container
+ E266 [*] Too many leading `#` before block comment
+ E265 [*] Block comment should start with `# `
+ PLR1730 [*] Replace `if` statement with `McMy = max(McMy, 1.0)`
+ PLR1730 [*] Replace `if` statement with `McMy = min(McMy, 1.3)`
+ PLR1730 [*] Replace `if` statement with `Cpr = min(1.2, Cpr)`
+ PLR1730 [*] Replace `if` statement with `ro_req = max(ro_req, ro_min_b)`
+ PLR1730 [*] Replace `if` statement with `maxDriftPiso = max(maxDriftPiso, drift_piso)`
+ E231 [*] Missing whitespace after ','
+ E262 [*] Inline comment should start with `# `
+ PLR1730 [*] Replace `if` statement with `max` call
+ FURB152 [*] Replace `3.14` with `math.pi`
+ PLR1730 [*] Replace `if` statement with `tank_level = max(tank_level, 0)`
+ PLR1730 [*] Replace `if` statement with `tank_level = max(tank_level, cur_node.min_level)`
+ PLR1730 [*] Replace `if` statement with `min_step_time = min(min_step_time, time_step)`
+ PLR1730 [*] Replace `if` statement with `av_r = min(av_r, 1)`
- FURB140 [*] Use `itertools.starmap` instead of the generator
- PLR0203 [*] Static method defined without decorator
  1. I applied those fixes on a disposable branch to get diffs.
  2. I inspected the diffs. All fixes in the above block marked with + appear appropriate. For the last two marked with a - I wasn't sure if they were causing a breaking check (nor did I check), so I marked the changes for reversal.
  3. I created a branch starting from the latest master branch and manually reverted the potentially unsafe changes.
  4. Most importantly, I updated the pyproject.toml configuration file to disable automatic fixes for rules with preview status.

A PR will follow with updates to the codespell configuration.

Commands used:
```
ruff format
ruff check --add-noqa
ruff format  # 331 files left unchanged
ruff check  # All checks passed!
```
@ioannis-vm
Copy link
Contributor Author

Note

PR #304 takes care of the spell-checking workflow.

@fmckenna fmckenna merged commit 125b166 into NHERI-SimCenter:master Aug 15, 2024
2 of 4 checks passed
@ioannis-vm ioannis-vm deleted the FIX_preview_rules branch August 17, 2024 10:54
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