Skip to content
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.

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

DOC: add warning to allclose, revise "Notes" in isclose #24407

Merged
merged 4 commits into from Dec 30, 2023

Conversation

OverLordGoldDragon
Copy link
Contributor

See #10161 (latest comments, ~8/13/2023)

  1. Add isclose's warning to allclose
  2. Revise isclose's "Notes" on atol to emphasize its poorness and better explain its purpose
  3. Add revised isclose note to allclose

See numpy#10161 (latest comments, ~8/13/2023)

1: Add `isclose`'s warning to `allclose`

2: Revise `isclose`'s "Notes" on `atol` to emphasize its poorness and better
explain its purpose

3: Add revised `isclose` note to `allclose`
@mdhaber
Copy link
Contributor

mdhaber commented Dec 29, 2023

@OverLordGoldDragon if you are interested in continuing with this, I can review it. The merge conflicts would need to be resolved. Would you like to do that, or would you like me to take care of it and push the change to your branch?

@mdhaber
Copy link
Contributor

mdhaber commented Dec 30, 2023

I took the liberty of resolving the conflicts and pushing the change to your branch.

I made a few adjustments.

  • The text "atol is used exclusively for determining whether a non-zero value in a is close to a zero value in b" could be misinterpreted as meaning that atol is only used when there is a non-zero value in a and a zero value in b, so I modified it.
  • The statement " If the expected input values are significantly smaller than one, it can result in false positives." seemed redundant given the example, so I removed it.
  • The original statement "atol=0 will result in False if either a or b contains a zero." was not perfectly precise: if both a and b are exactly 0, the result is True. In any case, this didn't seem like either a very subtle or particularly notable consequence of the definition, so I removed it.

If the rendered docs look good, I'll merge when CI finishes.

numpy/_core/numeric.py Outdated Show resolved Hide resolved
@mdhaber mdhaber merged commit 65e6c39 into numpy:main Dec 30, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants