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
BUG, ENH: fix array2string rounding bug by adding min_digits option #18629
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
def format_float_scientific( | |
x: _FloatLike_co, | |
precision: Optional[int] = ..., | |
unique: bool = ..., | |
trim: Literal["k", ".", "0", "-"] = ..., | |
sign: bool = ..., | |
pad_left: Optional[int] = ..., | |
exp_digits: Optional[int] = ..., | |
) -> str: ... | |
def format_float_positional( | |
x: _FloatLike_co, | |
precision: Optional[int] = ..., | |
unique: bool = ..., | |
fractional: bool = ..., | |
trim: Literal["k", ".", "0", "-"] = ..., | |
sign: bool = ..., | |
pad_left: Optional[int] = ..., | |
pad_right: Optional[int] = ..., | |
) -> str: ... |
b510396
to
ed3a024
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net>
Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net>
Could use a release note. |
Thanks Allan |
Fixes #18609
This PR adds a new
min_digits
argument 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=True
mode, even if the extra digits are unnecessary to uniquely specify the value. It is the counterpart to theprecision
argument which sets the maximum number of digits to be printed. (Whenunique=False
fixed-precision mode, it has no effect, and theprecision
arg 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=True
because 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_digits
andprecision
arguments to fix the number of output digits.It looks like a lof of chages, but a lot of it is just introducing the
min_digits
arg into all the function signatures in dragon4 and in the user api.