Skip to content

Fix new ruff errors - #179

Merged
inducer merged 3 commits into
inducer:mainfrom
alexfikl:fix-ruff
Aug 28, 2026
Merged

Fix new ruff errors#179
inducer merged 3 commits into
inducer:mainfrom
alexfikl:fix-ruff

Conversation

@alexfikl

Copy link
Copy Markdown
Collaborator

No description provided.

Comment on lines 623 to 627
def test_oscillatory():
# Oscillatory
approx_int, err_est = quadgk(lambda x: np.sin(x), 0, np.pi)
approx_int, err_est = quadgk(np.sin, 0, np.pi)
assert err_est < 1e-10
check("integral sin(x) from 0 to pi", approx_int, 2.0)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Are these tests here on purpose? Shouldn't they be in modepy/test?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

They are! I noticed this pattern in Rust code, where tests live alongside the code being tested, and rather liked it. At least for the packages where the tests live under the main package import, they're also guaranteed to be discovered. Do you not like this?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Hm.. I don't know, can't say I have too much of a preference. The main worry is just consistency, I guess? i.e. I wouldn't have looked here for tests.

Although I'm not a big fan of these being exported by default. Does pytest not find them if they're not in __all__?

@inducer
inducer merged commit 9008594 into inducer:main Aug 28, 2026
9 checks passed
@inducer

inducer commented Aug 28, 2026

Copy link
Copy Markdown
Owner

Thx!

@alexfikl
alexfikl deleted the fix-ruff branch August 29, 2026 06:59
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