8000 ENH: add parameter `strict` to `assert_allclose` by mdhaber · Pull Request #24680 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

ENH: add parameter strict to assert_allclose #24680

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Sep 21, 2023
Merged

ENH: add parameter strict to assert_allclose #24680

merged 3 commits into from
Sep 21, 2023

Conversation

mdhaber
Copy link
Contributor
@mdhaber mdhaber commented Sep 10, 2023 •
edited
Loading

This PR adds the parameter strict to np.testing.assert_allclose. With strict=True, the shape and dtype of the two arguments must match for the assertion to pass.

Follows the example of gh-21595
Partially addresses suggestion in #24667 (comment)

@mdhaber mdhaber mentioned this pull request Sep 10, 2023
Merged
mdhaber
mdhaber commented Sep 10, 2023
View reviewed changes
Copy link
Contributor Author
@mdhaber mdhaber left a comment

Choose a reason for hiding this comment

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

To fix the CircleCI failure.

@mdhaber
8000 Copy link
Contributor Author
mdhaber commented Sep 11, 2023 •
edited
Loading

IIUC, I should add a file doc/release/upcoming_changes/24680.new_feature.rst with contents like

``strict`` option for `testing.assert_allclose`
-----------------------------------------------
The ``strict`` option is now available for `testing.assert_allclose`.
Setting ``strict=True`` will disable the broadcasting behaviour for scalars and
ensure that input arrays have the same data type.

? I'll also commit the <BLANKLINE>s above and run CI with [skip actions] [skip cirrus] [skip azp]?

@seberg
Copy link
Member
seberg commented Sep 11, 2023

That seems about right, yes.

@mdhaber
MAINT: fix doc build issue; add new_feature.rst
a42827c 
[skip actions] [skip cirrus] [skip azp]
mdhaber
mdhaber commented Sep 11, 2023
View reviewed changes
Copy link
Contributor Author
@mdhaber mdhaber left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Two minor tweaks for the reviewer to consider. Feel free to commit.
When this looks good, I'll submit a follow-up for assert_equal, if that would be helpful?
These will help address #24667 (comment).

@tylerjereddy tylerjereddy mentioned this pull request Sep 11, 2023
Merged
@mattip
Copy link
Member
mattip commented Sep 21, 2023

@mdhaber Please go ahead and commit your self-review changes so we can get this in.

@mdhaber
DOC: fix intersphinx reference; replace "pi" with symbol
9dc5865 
[skip actions] [skip cirrus] [skip azp]
@mdhaber
Copy link
Contributor Author
mdhaber commented Sep 21, 2023 •
edited
Loading

Thanks @mattip. That rendered as intended.
When this merges, shall I do the same for assert_equal?

@mattip mattip merged commit 224b28f into numpy:main Sep 21, 2023
@mattip
Copy link
Member
mattip commented Sep 21, 2023

Thanks @mdhaber. Yes, I think assert_equal could also get a strict kwarg. At the same time, the docstring of assert_equal could use some touchup: it is missing the Notes and See Also that the others have.

@mdhaber mdhaber mentioned this pull request Sep 21, 2023
Merged
@mdhaber
Copy link
Contributor Author
mdhaber commented Sep 30, 2023

@mattip I opened gh-24770 to add the strict kwarg to assert_equal and touch up the docs as you suggested. gh-24775 adds strict to assert_array_less.

@h-vetinari h-vetinari mentioned this pull request Mar 11, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
01 - Enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mdhaber @seberg @mattip
0