MAINT: lint: enable UP rules #19516

lucascolley · 2023-11-13T18:56:01Z

Reference issue

What does this implement/fix?

The UP rules are enabled in the linter
All existing violations are cleaned up

The Ruff PyUpgrade rules ensure that SciPy uses modern Python constructs that work with all support Python versions.
While the linter only runs on files touched in any given PR, we have the option to apply these rules wholesale each time we drop support for another version of Python.
This keeps our code current, at low cost to maintainers.

Additional information

The vast majority of these changes were made automatically by Ruff's --fix, with the rest (changing from Union typing and to yield from) made by me.

Due to the automatic fixes, further (minor) style cleanups are possible here, but I don't know if they're worth the effort (or worth delaying this PR for).

The blame ignore will be added once this is ready.

stefanv · 2023-11-13T19:17:41Z

If there is agreement on this change, we should add a commit with an entry in .git-blame-ignore-revs, and then merge (not squash merge) the PR.

lucascolley · 2023-11-13T19:20:48Z

If there is agreement on this change, we should add a commit with an entry in .git-blame-ignore-revs, and then merge (not squash merge) the PR.

Yes, I didn't yet in anticipation of a force push (which was immediately necessary since I forgot to mamba upgrade ruff). Perhaps there will be no force pushes from now on but no harm in holding off for a little longer.

lucascolley · 2023-11-14T12:28:14Z

CI is green (apart from the broken docs build). I've noticed some formatting issues (multiline strings off by 1 space, closing brackets on a new line). These will probably have to be looked at manually unfortunately.

ilayn · 2023-11-14T12:53:34Z

These will probably have to be looked at manually unfortunately.

It would difficult to catch them later if we lose the .format() style. Because then the tool will not complain about anything.

I see quite a number of

    ('some long string'
    '')   # <- This looks like a tool bug to me

instances and I think ruff should be fixed about that.

lucascolley · 2023-11-14T13:04:02Z

This looks like a tool bug to me

IIUC this is because the linter is intended to be used in tandem with a formatter (i.e. linter is minimal in formatting changes). This is difficult for SciPy to adopt short of formatting the entire codebase (far too disruptive).

Edit: I'm happy to do a full pass through the diff to fix these. I'm quite busy right now so it will take a little while.

melissawm · 2023-11-15T14:23:57Z

Docs build failure is fixed in main, not related to this PR.

mdhaber

I skimmed this, mostly trusting that the automated linting didn't make any logical errors. I noted a few of the ugly things I found toward the top, but they are quite common, so I didn't continue, since we are supposed to rush this. Feel free to ignore these suggestions because they're not nearly complete.

~~The one thing I'll ask is why the lint check is passing, because some of these lines are far longer than the limit. Is that check not enabled here?~~

I thought this was related to the line length stuff, but I guess not. We will need a separate pass for that, I suppose.

scipy/_build_utils/system_info.py

scipy/_lib/decorator.py

scipy/constants/_codata.py

scipy/fft/_pocketfft/basic.py

scipy/interpolate/_fitpack_impl.py

scipy/ndimage/_measurements.py

scipy/odr/_odrpack.py

scipy/optimize/_shgo_lib/_complex.py

scipy/stats/tests/test_distributions.py

lucascolley · 2023-11-16T00:44:30Z

Thanks Matt, I added the needs-work label since I intend to do one full pass for style myself.

lucascolley · 2023-11-16T08:40:34Z

astral-sh/ruff#8683 at least some of the formatting issues will be fixed in the next ruff release. I might wait for that and remake the PR (as long as a force push wouldn't be too disruptive?)

I'm also inclined to wait for gh-19529 in case anything else arises from there.

stefanv · 2023-11-16T17:40:21Z

@lucascolley In the interest of time, consider using ruff from their main branch?

lucascolley · 2023-11-16T19:55:43Z

down to 22 errors which need to be fixed manually. There may be more once I rebase for gh-19529.

lucascolley · 2023-11-16T20:36:37Z

A slight change is needed for mypy from CI but I'lll wait until I've rebased on gh-19529 and fixed any additional errors. After that, I'll do a pass for style and check if any of the old comments still apply, then mark as ready for review. If everything looks good then, we may want to condense down to two commits for the purpose of the blame-ignore.

lucascolley · 2023-11-19T13:00:29Z

@j-bowhay @stefanv would either of you like to give this a look? No rush at all but I think it's ready. The safe auto fixes should be fine so perhaps just check the unsafe ones and my manual fixes?

j-bowhay

Changes seem reasonable but might be useful for another pair of eyes as it is easy to miss stuff

j-bowhay · 2023-11-19T13:36:56Z

scipy/optimize/_zeros_py.py

            raise ValueError("f(a) and f(b) must have different signs, but "
-                             "f({:e})={:e}, f({:e})={:e} ".format(a, fa, b, fb))
+                             f"f({a:e})={fa:e}, f({b:e})={fb:e} ")


Is the alignment here correct?

Ah no, good spot 👍

On second thought I think this is correct.

stefanv · 2023-11-21T00:46:03Z

OK, let's get this in before it goes stale. Do you want to make the .git-blame-ignore-revs commit(s)?

lucascolley · 2023-11-21T01:06:16Z

OK, let's get this in before it goes stale. Do you want to make the .git-blame-ignore-revs commit(s)?

👍 I'll address Jake's comment, squash down to 1 clean-up commit and add it to the ignored revisions (tomorrow). Thanks all for the quick review!

lucascolley · 2023-11-21T13:20:59Z

@stefanv good to go once CI is okay?

stefanv · 2023-11-21T18:18:32Z

I see the same failure on PRs recently merged to main, so let's get this in before it becomes impossible to merge.

stefanv · 2023-11-21T18:19:35Z

Thanks very much @lucascolley.

lucascolley · 2023-11-21T18:56:50Z

thanks all!

I imagine there isn't appetite for any more big changes soon, but if there are any other rules which you think may be worth our time @stefanv, let me know and I'm happy to look into them.

lucascolley requested review from larsoner, rgommers, person142, steppi, tylerjereddy, peterbell10, perimosocordiae, ilayn, andyfaff and ev-br as code owners November 13, 2023 18:56

lucascolley force-pushed the lint-UP branch from 8a75596 to 55f4c82 Compare November 13, 2023 19:10

j-bowhay added the maintenance Items related to regular maintenance tasks label Nov 14, 2023

lucascolley mentioned this pull request Nov 14, 2023

query: UP032 fix leaves bad formatting? astral-sh/ruff#8683

Closed

lucascolley added needs-work Items that are pending response from the author needs-decision Items that need further discussion before they are merged or closed labels Nov 15, 2023

mdhaber approved these changes Nov 15, 2023

View reviewed changes

mdhaber marked this pull request as draft November 16, 2023 14:02

lucascolley force-pushed the lint-UP branch from 55f4c82 to a9299b6 Compare November 16, 2023 19:54

lucascolley removed the needs-work Items that are pending response from the author label Nov 16, 2023

MAINT: lint: enable UP rules

d1d4f4b

lucascolley force-pushed the lint-UP branch 2 times, most recently from 1df50b1 to 213ce8d Compare November 16, 2023 21:48

lucascolley marked this pull request as ready for review November 16, 2023 21:51

lucascolley requested a review from mdhaber November 16, 2023 21:51

lucascolley mentioned this pull request Nov 16, 2023

STY: comply with the documented line length limit #19479

Closed

j-bowhay approved these changes Nov 19, 2023

View reviewed changes

lucascolley removed the needs-decision Items that need further discussion before they are merged or closed label Nov 21, 2023

lucascolley added 2 commits November 21, 2023 13:18

MAINT: lint: clean-up for UP rules

9cf72e4

MAINT: blame-ignore for UP rules clean-up

3b631db

lucascolley force-pushed the lint-UP branch from 213ce8d to 3b631db Compare November 21, 2023 13:20

stefanv merged commit 731b325 into scipy:main Nov 21, 2023
19 of 22 checks passed

j-bowhay added this to the 1.12.0 milestone Nov 21, 2023

lucascolley deleted the lint-UP branch November 21, 2023 19:19

lucascolley mentioned this pull request Apr 27, 2024

ENH: stats.circ___: add array-API support #20595

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MAINT: lint: enable UP rules #19516

MAINT: lint: enable UP rules #19516

lucascolley commented Nov 13, 2023 •

edited

stefanv commented Nov 13, 2023

lucascolley commented Nov 13, 2023

lucascolley commented Nov 14, 2023

ilayn commented Nov 14, 2023

lucascolley commented Nov 14, 2023 •

edited

melissawm commented Nov 15, 2023

mdhaber left a comment •

edited

lucascolley commented Nov 16, 2023

lucascolley commented Nov 16, 2023 •

edited

stefanv commented Nov 16, 2023

lucascolley commented Nov 16, 2023

lucascolley commented Nov 16, 2023

lucascolley commented Nov 19, 2023

j-bowhay left a comment

j-bowhay Nov 19, 2023

lucascolley Nov 19, 2023

lucascolley Nov 21, 2023

stefanv commented Nov 21, 2023

lucascolley commented Nov 21, 2023

lucascolley commented Nov 21, 2023

stefanv commented Nov 21, 2023

stefanv commented Nov 21, 2023

lucascolley commented Nov 21, 2023 •

edited

MAINT: lint: enable UP rules #19516

MAINT: lint: enable UP rules #19516

Conversation

lucascolley commented Nov 13, 2023 • edited

Reference issue

What does this implement/fix?

Additional information

stefanv commented Nov 13, 2023

lucascolley commented Nov 13, 2023

lucascolley commented Nov 14, 2023

ilayn commented Nov 14, 2023

lucascolley commented Nov 14, 2023 • edited

melissawm commented Nov 15, 2023

mdhaber left a comment • edited

Choose a reason for hiding this comment

lucascolley commented Nov 16, 2023

lucascolley commented Nov 16, 2023 • edited

stefanv commented Nov 16, 2023

lucascolley commented Nov 16, 2023

lucascolley commented Nov 16, 2023

lucascolley commented Nov 19, 2023

j-bowhay left a comment

Choose a reason for hiding this comment

j-bowhay Nov 19, 2023

Choose a reason for hiding this comment

lucascolley Nov 19, 2023

Choose a reason for hiding this comment

lucascolley Nov 21, 2023

Choose a reason for hiding this comment

stefanv commented Nov 21, 2023

lucascolley commented Nov 21, 2023

lucascolley commented Nov 21, 2023

stefanv commented Nov 21, 2023

stefanv commented Nov 21, 2023

lucascolley commented Nov 21, 2023 • edited

lucascolley commented Nov 13, 2023 •

edited

lucascolley commented Nov 14, 2023 •

edited

mdhaber left a comment •

edited

lucascolley commented Nov 16, 2023 •

edited

lucascolley commented Nov 21, 2023 •

edited