Apply monthly pre-commit update manually#17843
Conversation
The manual updates allow implementing better fixes to Ruff rule FURB188 (slice-to-remove-prefix-or-suffix) in two files in `io/fits`.
eerovaher
requested review from
a team,
WilliamJamieson,
dhomeier,
mcara,
nstarman,
saimn and
taldcroft
as code owners
github-actions
bot
added
testing
io.votable
wcs
table
io.fits
config
time
io.ascii
nddata
units
modeling
utils
skip-changelog-checks
|
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
| tform = compressed_coldefs[column_name].format | ||
| if tform.startswith("1"): | ||
| tform = tform[1:] | ||
| tform = compressed_coldefs[column_name].format.removeprefix("1") |
There was a problem hiding this comment.
The automatic Ruff update would have been
tform = compressed_coldefs[column_name].format
- if tform.startswith("1"):
- tform = tform[1:]
+ tform = tform.removeprefix("1")
| keyword = keyword.strip().upper() | ||
| if keyword.startswith("HIERARCH "): | ||
| keyword = keyword[9:] | ||
| keyword = keyword.strip().upper().removeprefix("HIERARCH ") |
There was a problem hiding this comment.
The automatic Ruff update would have been
keyword = keyword.strip().upper()
- if keyword.startswith("HIERARCH "):
- keyword = keyword[9:]
+ keyword = keyword.removeprefix("HIERARCH ")
|
Thanks. I see the diff count is slightly different here than on #17841, can you explain where are the two additional lines you removed ? |
dhomeier
left a comment
dhomeier
left a comment
There was a problem hiding this comment.
Thanks, io.ascii (and from a quick glimpse, the rest of io) looking good!
| lim_vals = ( | ||
| f"[{floor(col.min * 100) / 100.}/{ceil(col.max * 100) / 100.}]" | ||
| ) | ||
| lim_vals = f"[{floor(col.min * 100) / 100.0}/{ceil(col.max * 100) / 100.0}]" |
There was a problem hiding this comment.
As long as I don't need to understand why Ruff is now undoing formatting that was introduced by Black rules in the first place, definitely +1 on this!
There was a problem hiding this comment.
black's format isn't immutable either: it's updated once a year. Ruff follows a similar schedule and both seems to be introducing identical or very similar changes so far.
There was a problem hiding this comment.
So it's also changing the default line length again between versions?
Personally I'm all in favour of the longer limits, and the fewer = ($ constructs, the better, but I thought we had discussed at some length (pun inevitable) that the then-black-default of 88 was preferable to most.
There was a problem hiding this comment.
No, the default is still 88. Ruff is just getting intentionally looser on this specific rule. I don't know which specific rule change causes this, but I think it makes perfect sense to forfeit formatting on such a line: there is no safe way to split an arbitrary string, and putting parens doesn't guarantee that the limit will be respected in the general case any way.
There was a problem hiding this comment.
There was a problem hiding this comment.
Thanks, just what I had been looking for, but searching through the entire changelog did not bring up anything related. Though I still don't understand how that rule applies here, since it the f-string part of that expression had never been split in the first place, but I guess I don't have to.
|
|
Thanks, all! |
5 participants
Description
The manual updates allow implementing better fixes to Ruff rule FURB188 (slice-to-remove-prefix-or-suffix) in two files in
io.*. Note that #17619 and #17620 were opened almost two months ago with the purpose of making the patches thepre-commitautoupdates would apply smaller, but they still haven't been merged. Given thatpre-commithas already tried to apply its monthly updates twice since their opening, they are no longer valuable as separate pull requests.Closes #17615, closes #17619, closes #17620, closes #17841
edit by @neutrinoceros: use the correct path for where files with FURB188 are.