Fix flake8 bugbear - #14692

Could you point us to some background info which toolset made these changes and why, like is this a change to our current ruff setup?

Could you point us to some background info which toolset made these changes and why, like is this a change to our current ruff setup?

Yes, this is removing some ignores from our ruff config. This is for the flake8-bugbear rule set.

• B004, Using hasattr(x, '__call__') to test if x is callable is unreliable. Use callable(x) for consistent results.
• B028, Checks for warnings.warn calls without an explicit stacklevel keyword argument. "The warnings.warn method uses a stacklevel of 1 by default, which limits the rendered stack trace to that of the line on which the warn method is called. It's recommended to use a stacklevel of 2 or higher, give the caller more context about the warning." https://beta.ruff.rs/docs/rules/no-explicit-stacklevel/
• B024, an abstract base class has no abstract methods (and shouldn't be abstract).
• B018, found a useless expression. Either assign it to a variable or remove it.
• B020, a loop control variable overrides the iterable it iterates. This is dangerous and hard to debug.
• B026, found func(x=..., *args) which is very dangerous and only works if x is actually the first argument.
• B005, "Using .strip() with multi-character strings is misleading the reader. It looks like stripping a substring. Move your character set to a constant if this is deliberate. Use .replace(), .removeprefix(), .removesuffix() or regular expressions to remove string fragments." https://pypi.org/project/flake8-bugbear/

Yes, this is removing some ignores from our ruff config. This is for the flake8-bugbear rule set.

Thanks; it's becoming a bit difficult to follow up on which tools we have decided to use, and with which settings. Hopefully this can be discussed at the meeting next week.

I agree that changes affecting functionality like B028 (some of the others above may fall into that category as well, not sure) should be committed separately. It seems, as @saimn's example shows, which stacklevel is most helpful depends quite on the scope of the immediate caller function (and possibly those in the levels above), and would need closer case-by-case inspection (especially if B028 means setting blanket stacklevel=2 everywhere).

It seems, as @saimn's example shows, which stacklevel is most helpful depends quite on the scope of the immediate caller function (and possibly those in the levels above), and would need closer case-by-case inspection (especially if B028 means setting blanket stacklevel=2 everywhere).

I agree. I've removed that commit to tackle in a future PR.

I also wondered about just ignoring B018 altogether, but you probably got the right balance here, by ignoring it in test files. Other changes all look good.

Thanks @eerovaher & @mhvk for the reviews!
Giving that #14744 is coming, I'd love to get this in. @WilliamJamieson @pllim @mhvk for getting an approval critical mass.

Looks great! Indeed, for timeseries/binned.py, this corrects an actual buglet.

Was that once valid in some Python version?

Re: #14692 (comment)

I think with this change, format can no longer be passed in as positional argument. 🤷

It is OK - format is just being passed on, and the change ensures that other positional arguments can be passed on as well (though obviously they are rarely needed, since nobody found the error this would have produced...)