BUG, ENH: fix array2string rounding bug by adding min_digits option#18629
Merged
charris merged 5 commits intonumpy:mainfrom Mar 31, 2021
Merged
BUG, ENH: fix array2string rounding bug by adding min_digits option#18629charris merged 5 commits intonumpy:mainfrom
charris merged 5 commits intonumpy:mainfrom
Conversation
BvB93
reviewed
Mar 17, 2021
Member
BvB93
left a comment
There was a problem hiding this comment.
Can you update the annotations in the arrayprint.pyi stub file?
By the looks of it the type of min_digits should be Optional[int].
numpy/numpy/core/arrayprint.pyi
Lines 98 to 116 in ee9c8f8
b510396 to
ed3a024
Compare
seberg
approved these changes
Mar 19, 2021
Member
seberg
left a comment
There was a problem hiding this comment.
Thanks Allan. I took the liberty to rebase, if we want a backport give me a ping and I will do that.
I won't claim the have followed all the logic in dragon4.c but the code changes look all good and the tests sufficiently thorough.
seberg
reviewed
Mar 19, 2021
Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net>
Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net>
Member
|
Could use a release note. |
charris
reviewed
Mar 26, 2021
charris
reviewed
Mar 26, 2021
Member
|
Thanks Allan |
charris
added a commit
to charris/numpy
that referenced
this pull request
Mar 31, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #18609
This PR adds a new
min_digitsargument to the dragon4 float printing methods, such asnp.format_float_positional. This kwd guarantees that at least the given number of digits will be printed, when printing inunique=Truemode, even if the extra digits are unnecessary to uniquely specify the value. It is the counterpart to theprecisionargument which sets the maximum number of digits to be printed. (Whenunique=Falsefixed-precision mode, it has no effect, and theprecisionarg fixes the number of digits)Using this new keyword, I then fix a bug in arrayprint ( #18609). The bug in the old code is that, in order to guarantee that all elements of an array print using the same number of digits, we turned off
unique=Truebecause we needed to use fixed-precision mode to fix the number of digits. However, unique mode rounds differently from fixed-precision mode, so the rounding was wrong. This PR instead now stays in unique mode to print the elements, but uses themin_digitsandprecisionarguments to fix the number of output digits.It looks like a lof of chages, but a lot of it is just introducing the
min_digitsarg into all the function signatures in dragon4 and in the user api.