Skip to content

fix: pass atol, rtol and equal_nan as kwargs to awkward's isclose method#658

Merged
pfackeldey merged 2 commits intoscikit-hep:mainfrom
mrzimu:mrli/fix-isclose-argument-for-awkward
Nov 3, 2025
Merged

fix: pass atol, rtol and equal_nan as kwargs to awkward's isclose method#658
pfackeldey merged 2 commits intoscikit-hep:mainfrom
mrzimu:mrli/fix-isclose-argument-for-awkward

Conversation

@mrzimu
Copy link
Contributor

@mrzimu mrzimu commented Oct 30, 2025

Description

Hi,
This PR fixes #657, not only for Vector3D, but also 2D and lorentz vector. I also added a test for this.

Checklist

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't any other open Pull Requests for the required change?
  • Does your submission pass pre-commit? ($ pre-commit run --all-files or $ nox -s lint)
  • Does your submission pass tests? ($ pytest or $ nox -s tests)
  • Does the documentation build with your changes? ($ cd docs; make clean; make html or $ nox -s docs)
  • Does your submission pass the doctests? ($ pytest --doctest-plus src/vector/ or $ nox -s doctests)

Before Merging

  • Summarize the commit messages into a brief review of the Pull request.

Copy link
Collaborator

@pfackeldey pfackeldey left a comment

Choose a reason for hiding this comment

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

Thanks @mrzimu, looks good to me! 🎉

I have only one request: could you move the test functions into this file https://github.com/scikit-hep/vector/blob/main/tests/test_issues.py? It's a dedicated test file that contains test for issues opened in GitHub and the test function name contains the issue number. In your case that would be def test_issue_657():. This lets us track down the test to the issue in the future :)

Copy link
Member

@Saransh-cpp Saransh-cpp left a comment

Choose a reason for hiding this comment

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

Thanks, @mrzimu! This looks great! @pfackeldey please feel free to merge once you approve ✅

Copy link
Collaborator

@pfackeldey pfackeldey left a comment

Choose a reason for hiding this comment

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

LGTM! thanks @mrzimu for the PR and @Saransh-cpp for the review 🎉 I'll merge this one 👍

@pfackeldey pfackeldey merged commit fbf94af into scikit-hep:main Nov 3, 2025
15 checks passed
@mrzimu mrzimu deleted the mrli/fix-isclose-argument-for-awkward branch November 3, 2025 07:57
@mrzimu
Copy link
Contributor Author

mrzimu commented Nov 3, 2025

Yeah, happy to see this helps :)

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.

isclose method for ak.Record & Vector3D breaks for v1.7.0

3 participants