Skip to content

[chore] Stricter ruff rules#282

Open
fjosw wants to merge 7 commits into
developfrom
ruff_rules_strict
Open

[chore] Stricter ruff rules#282
fjosw wants to merge 7 commits into
developfrom
ruff_rules_strict

Conversation

@fjosw

@fjosw fjosw commented Apr 20, 2026

Copy link
Copy Markdown
Owner

This PR imposes stricter ruff rules and fixes the parts which raise errors now. Main changes:

@fjosw fjosw marked this pull request as ready for review April 20, 2026 17:49

@jkuhl-uni jkuhl-uni left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm

@jkuhl-uni

Copy link
Copy Markdown
Collaborator

Thank you for all the work on the code style!
I think having these slightly stricter rules makes sense. I have to familiarize myself with some of the reasoning behind the ruff rules, but I think it is a good idea overall.

@s-kuberski

Copy link
Copy Markdown
Collaborator

Hi! Thanks for taking the time to implement this! I am still in the process of looking through the changes.

I was wondering about the change of the handling of random numbers. I understand that switching to np.random.default_rng() is considered to be the proper modern solution. However, I am asking myself if this change, hidden inside the large number of other changes, could lead to unexpected behavior.

As I understand it, changing the numpy random seed will not affect the state of the random number generator anymore, right? So there won't be any way to fix the seed in the random number generator in the future, when its initialization is hidden in the module. Could this lead to problems?

fjosw added 3 commits June 18, 2026 10:46
Restore use of the global np.random state in pseudo_Obs (misc.py) and the
prior id generation (fits.py), keeping seed behavior unchanged. The switch
to a module-local generator is out of scope for this lint-focused PR.
The RNG migration was reverted to keep np.random.seed() behavior, so add
per-line noqa: NPY002 on the three legacy np.random calls instead of the
Generator API.
@fjosw

fjosw commented Jun 18, 2026

Copy link
Copy Markdown
Owner Author

After quite some time I had another look at this PR. I agree with leaving the RNG changes out of this PR and potentially do them separately or not at all. I removed the rng changes from the diff. Could you review the changes that are left @s-kuberski @jkuhl-uni ?

@jkuhl-uni

Copy link
Copy Markdown
Collaborator

Hi,
thank you again for the work.
I think moving the changes to another PR is a good idea. I haven't had time to think more about the calls within NumPy. If the current call from pyerrors is considered legacy, I hope there is an alternative that would have the same properties in regard to the seed and instantiation of the RNG as the legacy call.

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.

3 participants