Skip to content

[MAINT] apply pre-commit autofixes#1468

Draft
Remi-Gau wants to merge 1 commit intonipy:masterfrom
Remi-Gau:apply/precommit
Draft

[MAINT] apply pre-commit autofixes#1468
Remi-Gau wants to merge 1 commit intonipy:masterfrom
Remi-Gau:apply/precommit

Conversation

@Remi-Gau
Copy link
Contributor

  • related to [WIP][MAINT] bump pre-commit hooks versions #1467
  • simply run pre-commit on all files
  • mostly adds an explicit strict to all zip()
  • ignore a couple of other errors that would require manual fixes
  • adapts a conditional import that is always true since we are always on >= 3.10

Comment on lines +171 to +177
"RUF007",
"RUF012", # TODO: enable
"RUF015",
"RUF017", # TODO: enable
"UP007",
"UP038",
"TC003",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ignore those new errors for now, but I could either try to fix them in this PR or have side PRs for it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like UP038. It's churn for its own sake, and I've seen Python core devs regret making unions isinstance-able, since it doesn't work for many other type forms.

I think we should adopt UP007. The other two I don't care about one way or the other. If they have auto-fixes, fine, but I'm okay with ignoring as well.

Feel free to include in this PR if you've got the time/energy, but I'm okay with modular PRs done as time permits.

@codecov
Copy link

codecov bot commented Feb 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.42%. Comparing base (673962f) to head (9bf33e7).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1468   +/-   ##
=======================================
  Coverage   95.42%   95.42%           
=======================================
  Files         209      209           
  Lines       29822    29820    -2     
  Branches     4483     4482    -1     
=======================================
  Hits        28457    28457           
+ Misses        930      929    -1     
+ Partials      435      434    -1     

☔ 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.

Copy link
Member

@effigies effigies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose that we should prefer strict=True when there is no good reason for the lengths to differ? Or is it more efficient to use strict=False, and so we only prefer strict=True if they can differ and should error?

I strongly suspect that most could be strict, but here are a few that I have verified manually.

def items(self):
"""Return items from structured data"""
return zip(self.keys(), self.values())
return zip(self.keys(), self.values(), strict=False)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should never be different.

Suggested change
return zip(self.keys(), self.values(), strict=False)
return zip(self.keys(), self.values(), strict=True)

['right', 'center'],
]
for pos, anchor, lab in zip(poss, anchors, label):
for pos, anchor, lab in zip(poss, anchors, label, strict=False):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for pos, anchor, lab in zip(poss, anchors, label, strict=False):
for pos, anchor, lab in zip(poss, anchors, label, strict=True):

ax.set_xlim(x[0], x[-1])
yl = [self._data.min(), self._data.max()]
yl = [lim + s * np.diff(lims)[0] for lim, s in zip(yl, [-1.01, 1.01])]
yl = [lim + s * np.diff(lims)[0] for lim, s in zip(yl, [-1.01, 1.01], strict=False)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
yl = [lim + s * np.diff(lims)[0] for lim, s in zip(yl, [-1.01, 1.01], strict=False)]
yl = [lim + s * np.diff(lims)[0] for lim, s in zip(yl, [-1.01, 1.01], strict=True)]

idxs = np.dot(self._inv_affine, self._position)[:3]
idxs_new_order = idxs[self._order]
for ii, (size, idx) in enumerate(zip(self._sizes, idxs_new_order)):
for ii, (size, idx) in enumerate(zip(self._sizes, idxs_new_order, strict=False)):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for ii, (size, idx) in enumerate(zip(self._sizes, idxs_new_order, strict=False)):
for ii, (size, idx) in enumerate(zip(self._sizes, idxs_new_order, strict=True)):

self._sizes = [self._data.shape[order] for order in self._order]
for ii, xax, yax, ratio, label in zip(
[0, 1, 2], [1, 0, 0], [2, 2, 1], r, ('SAIP', 'SRIL', 'ARPL')
[0, 1, 2], [1, 0, 0], [2, 2, 1], r, ('SAIP', 'SRIL', 'ARPL'), strict=False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[0, 1, 2], [1, 0, 0], [2, 2, 1], r, ('SAIP', 'SRIL', 'ARPL'), strict=False
[0, 1, 2], [1, 0, 0], [2, 2, 1], r, ('SAIP', 'SRIL', 'ARPL'), strict=True

raise ValueError('voxel sizes should all be positive')
out_vox[:n_axes] = voxel_sizes
in_mn_mx = zip([0, 0, 0], np.array(in_shape) - 1)
in_mn_mx = zip([0, 0, 0], np.array(in_shape) - 1, strict=False)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
in_mn_mx = zip([0, 0, 0], np.array(in_shape) - 1, strict=False)
in_mn_mx = zip([0, 0, 0], np.array(in_shape) - 1, strict=True)

[ 2., 1.]])
"""
labels = list(zip('LPI', 'RAS')) if labels is None else labels
labels = list(zip('LPI', 'RAS', strict=False)) if labels is None else labels
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
labels = list(zip('LPI', 'RAS', strict=False)) if labels is None else labels
labels = list(zip('LPI', 'RAS', strict=True)) if labels is None else labels

"""
if labels is None:
labels = list(zip('LPI', 'RAS'))
labels = list(zip('LPI', 'RAS', strict=False))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
labels = list(zip('LPI', 'RAS', strict=False))
labels = list(zip('LPI', 'RAS', strict=True))

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