adopt ruff #353
adopt ruff #353
Conversation
|
Lots of style changes here. Is it worth adding a |
| only_use_cftime_datetimes=only_use_cftime_datetimes, | ||
| only_use_python_datetimes=only_use_python_datetimes, | ||
| ) | ||
| has_half_seconds = np.logical_and(time_units == "seconds", time_values % 1.0 == 0.5) |
There was a problem hiding this comment.
Seems like ruff is overriding black here. Maybe we shouldn't use both?
Lines 48 to 49 in cf4d66d
There was a problem hiding this comment.
Actually it looks like a lot of the changes in this PR are just increasing the allowed line length. Maybe it would be better to tell ruff to keep to 79 characters. I've no objection in general to an 88 character limit, but it might be good to separate concerns of tool choices vs style choices. Line limit could be increased as a follow-up PR.
There was a problem hiding this comment.
Looks like you just need a couple of lines in the pyproject.toml to reduce the character limit
[tool.ruff]
line-length = 79
I'd be interested to see how much smaller that makes the diff, and it might help inform the discussion at SciTools/iris#5254. Looks like a lot of the other changes are introducing f-strings to replace old-style string formatting, which seems fair enough.
There was a problem hiding this comment.
Seems like ruff is overriding black here. Maybe we shouldn't use both?
Lines 48 to 49 in cf4d66d
I've have wondered before whether we are using overlapping tools. I think there is scope for black to disagree with flake8 ?
There was a problem hiding this comment.
Ruff docs say they are compatible with black provided the line lengths are set the same.
https://beta.ruff.rs/docs/faq/#is-ruff-compatible-with-black
| "calendar": unit.calendar, | ||
| "only_use_cftime_datetimes": only_use_cftime_datetimes, | ||
| "only_use_python_datetimes": only_use_python_datetimes, | ||
| } |
There was a problem hiding this comment.
I'm curious about why this is the preferred form for specifying a dictionary.
There was a problem hiding this comment.
Huh. Maybe this is why
In [5]: %timeit x = {"a":1, "b":2}
70.2 ns ± 1.32 ns per loop (mean ± std. dev. of 7 runs, 10,000,000 loops each)
In [6]: %timeit x = dict(a=1, b=2)
119 ns ± 2.61 ns per loop (mean ± std. dev. of 7 runs, 10,000,000 loops each)
|
I really didn't want to get involved here but based on @rcomer's comments I think we need a team discussion. I think we can all agree that we don't want inconsistency between our repos, so adopting ruff for cf-units would therefore create an implicit goal to use it for all our repos, and that's a big decision. |
| ) | ||
| raise TypeError(emsg) |
There was a problem hiding this comment.
This refactor seems more clever than black 🤔
* config changes from #353 * keep existing line length and import sort order * remove flake8 config * Drop black; update ruff version and add formatter * ruff autofixes * ruff formatter fix * ruff 'unsafe' autofixes * Ruth fixes for unsafe fixes * re-run ruff formatter * Manually address E501 and B018 * point to astral-sh organisation; tidy some broken strings * review actions * update ruff-pre-commit version * fix pyproject.toml * fix pyproject.toml * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * ruff fixes * fix parantheses * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * more lint fixes * linting + pre-commit fixes * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * linting + pre-commit fixes * fix test * revert ruff fix * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * ignore generated code * remove redundant ignore --------- Co-authored-by: stephen.worsley <stephen.worsley@metoffice.gov.uk> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: stephenworsley <49274989+stephenworsley@users.noreply.github.com>
🚀 Pull Request
Description
This PR adopts ruff and drops the explicit use of
isortandflake8.Running
ruffacross the codebase rediscovers a whole collections of exceptions that clearly have been missed forflake8et al compliance.ruffbadge.