Skip to content

fix: replace fragile float == comparisons with pytest.approx in tests#56

Merged
jc-macdonald merged 5 commits intoyoavram-lab:mainfrom
nightcityblade:fix/issue-33
Apr 8, 2026
Merged

fix: replace fragile float == comparisons with pytest.approx in tests#56
jc-macdonald merged 5 commits intoyoavram-lab:mainfrom
nightcityblade:fix/issue-33

Conversation

@nightcityblade
Copy link
Copy Markdown

Fixes #33

Replaces bare float equality/inequality comparisons in tests with pytest.approx where the values come from computations, and adds comments for exact-by-construction comparisons.

Changes:

  • assert x == 0.1assert x == pytest.approx(0.1) for computed values
  • assert x == 0.0assert x == pytest.approx(0.0) for array element checks
  • Kept exact != comparisons where values are exact-by-construction (zero-fill, eps replacement) with explanatory comments
  • Removed RUF069 suppression from pyproject.toml

All 345 tests pass (1 pre-existing skip due to missing matplotlib).

@jc-macdonald jc-macdonald self-requested a review April 6, 2026 08:32
@jc-macdonald jc-macdonald self-assigned this Apr 6, 2026
@jc-macdonald
Copy link
Copy Markdown
Collaborator

Hello! Thanks for your interest in contributing to the package. As you can see some of the CI checks fail due to ruff compliance issues. If you look at the contributing page you will see our recommended path to auditing your code before submission. We recommend using uv/just for development work. There are a number of very helpful just recipes including just ci which will check your code for these sorts of issues prior to committing. Cheers!

Copy link
Copy Markdown
Collaborator

@jc-macdonald jc-macdonald left a comment

Choose a reason for hiding this comment

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

please run ruff checks and fix issues. Thanks for contributing!

@nightcityblade
Copy link
Copy Markdown
Author

Thanks, I checked this again locally. The files touched by this PR pass \All checks passed!, and I also ran a targeted pytest subset after installing the dev environment: \ (85 passed, 9 deselected). The repo still has unrelated pre-existing Ruff violations outside this PR, which is probably what CI was surfacing earlier.

@jc-macdonald
Copy link
Copy Markdown
Collaborator

Thanks for the follow-up! The CI failure was actually a formatting issue — our CI runs ruff format --preview which enforces the "hug parentheses" style (collapsing multi-line calls with a single dict/list argument). Your branch was based on an older main that didn't have those rules applied to the test files yet.

I've merged main into your branch and re-ran the formatter, so CI should be green now. Appreciate the contribution!

@jc-macdonald jc-macdonald self-requested a review April 8, 2026 21:13
Copy link
Copy Markdown
Collaborator

@jc-macdonald jc-macdonald left a comment

Choose a reason for hiding this comment

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

there were a few missed == fixes, some were added back by upstream. all are now fixed. Closes #33

@jc-macdonald jc-macdonald merged commit aabfaf9 into yoavram-lab:main Apr 8, 2026
6 checks passed
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.

Replace fragile float == comparisons in tests with pytest.approx / np.testing

2 participants