Ruff fixes

[A-CGray]

Purpose

Updates repo to pass our new formatting and linting checks. There are a lot of whitespace changes here so once this PR is merged we can add the commit hash to a .git-blame-ignore-revs file.

Expected time until merged

Type of change

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (non-backwards-compatible fix or feature)
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no API changes)
• Documentation update
• Maintenance update
• Other (please describe)

Testing

Checklist

• I have run ruff check and ruff format to make sure the Python code adheres to PEP-8 and is consistently formatted
• I have formatted the Fortran code with fprettify or C/C++ code with clang-format as applicable
• I have run unit and regression tests which pass locally with my changes
• I have added new tests that prove my fix is effective or that my feature works
• I have added necessary documentation

[codecov]

Codecov Report

❌ Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.22%. Comparing base (b9ff2c3) to head (5fcc7f7).

Files with missing lines	Patch %	Lines
pyoptsparse/pyALPSO/pyALPSO.py	0.00%	2 Missing ⚠️
pyoptsparse/pyOpt_constraint.py	0.00%	2 Missing ⚠️
pyoptsparse/testing/pyOpt_testing.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #455   +/-   ##
=======================================
  Coverage   86.22%   86.22%           
=======================================
  Files          24       24           
  Lines        3419     3419           
=======================================
  Hits         2948     2948           
  Misses        471      471           

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ewu63]

I really don't think we should be modifying the optimizer source code which we just vendor in - this large diff doesn't do anything for us.

[ewu63]

I'm also a little confused. How is the pre-commit supposed to be applied outside of CI? And is there a way to configure those per-repo? If the answer is no to either of those then we may want to rethink the pre-commit approach.

[A-CGray]

I really don't think we should be modifying the optimizer source code which we just vendor in - this large diff doesn't do anything for us.

I know it's a big diff, but we can put this PR's commit in .git-blame-ignore-revs. The kind of trailing whitespace fixes that are being applied to those source files are ones that many IDEs apply automatically, so doing this one off fix here avoids people unwittingly making these changes in the future.

I'm also a little confused. How is the pre-commit supposed to be applied outside of CI? And is there a way to configure those per-repo? If the answer is no to either of those then we may want to rethink the pre-commit approach.

Download the config file from the .github repo and put it in the repo in question (or put it somewhere central and softlink to it in the repo). pip install pre-commit, then you can manually run the checks with pre-commit run --all-files, or set them up to run on commits with pre-commit install. Extending pre-commit configs is explicitly not supported.

[ewu63]

The .git-blame-ignore-revs file doesn't actually do anything though, you have to set up your editor to accept this file. It's not actually clear to me if this can even be done on a per-repo level (checking if this particular file exists), see e.g. this question, and I think GitHub UI will not work with it either.

I think the checks that are included in pre-commit are OK for some repos, but I question whether it should be applied uniformly across everything -- our previous checks had the option of per-repo configuration which allowed us to adjust these settings. In the case of pyOptSparse, I really don't think it's worth the effort, as we're simply vendoring code from others.

Extending pre-commit configs is explicitly not supported.

This is by design (which I agree with) and that's what makes me think pre-commit is ill-suited for what we are trying to do. Shared configs is in direct conflict with the idea of running pre-commit in isolated envs. Plus, the convoluted (and undocumented) setup process makes it really hard for others to work on these projects, which might not matter for some repos but does matter for pyOptSparse.

[A-CGray]

I think the checks that are included in pre-commit are OK for some repos, but I question whether it should be applied uniformly across everything -- our previous checks had the option of per-repo configuration which allowed us to adjust these settings. In the case of pyOptSparse, I really don't think it's worth the effort, as we're simply vendoring code from others.

I don't really see it as any effort to apply these whitespace fixes here (in fact, the point was to avoid the potential effort of having to undo whitespace changes automatically made by IDEs in future). But I can remove the trailing-whitespace and end-of-file-fixer checks from our pre-commit config if you feel strongly about it.

This is by design (which I agree with) and that's what makes me think pre-commit is ill-suited for what we are trying to do. Shared configs is in direct conflict with the idea of running pre-commit in isolated envs.

I'm not sure I follow this point. Sharing configs is for consistency across repos, pre-commit running in an isolated env is for consistency between what users run locally and what we run in CI.

Plus, the convoluted (and undocumented) setup process makes it really hard for others to work on these projects, which might not matter for some repos but does matter for pyOptSparse.

I don't think the process for running the new checks locally is any more difficult than it was for black and flake8. In fact, in some sense it's easier because you don't need to do anything differently for repos that have a local linting config, like you did with flake8. I've opened a PR (mdolab/MACH-Aero#182) to update our contribution guide with new formatting and linting instructions.

[ewu63]

I think ultimately, my gripe is with the lack of configurability with this current setup. In the past, we had allowed each repo to define the set of rules that they would be compliant with (black, flake8, isort, pylint, etc.). Now, each repo is required to be compliant to all 10 rules stored here, and I think that causes unnecessary issues. Again, I think the vast majority of changes in this PR are somewhat unnecessary and difficult to review thoroughly. In addition to ruff, I'm fine with the mixed line ending changes.

In any case, I don't want this PR to block all development in pyOptSparse, I just don't think a +1304, -1310 PR of mostly whitespace changes is helpful.