Support gmpy2-like rounding modes in to_str() by javierelpianista · Pull Request #830 · mpmath/mpmath

javierelpianista · 2024-08-02T16:34:26Z

Closes #757.

* Added new rounding mode: round_nearest_even: round to nearest tying to even. * Modified round_digits to accomodate for this new mode.

* Wrote abs_rounding function. * Replaced part of the code in to_str with a call to round_digits. * Added tests for different kinds of rounding in test_convert.py

* Add new symbol, 'M', for rounding to nearest with tying away from zero. * Replaced part of the code in format_mpf with a call to abs_rounding * Added tests for the M rounding mode.

* Add new symbol, 'Y', for rounding away from zero. * Added tests for the Y rounding mode.

skirpichev

Thus,

~~please add 'Y' support in a separate pr~~; done: see #831
please don't introduce new rounding mode
rounding=None in to_str() will be current (legacy) behaviour, that should be deprecated
deprecation message should announce that new default will be round_nearest

mpmath/libmp/libmpf.py

* The default for the rounding parameter is None. * If rounding is None, a DeprecationWarning is raised; the rounding part of the code executes legacy code * Else, the new round_digits function is called

* Made all the dunder methods that call to_str to include the rounding parameter obtained from context. This is to avoid the new deprecation warning in to_str.

Modified the call to to_str in mpc_to_str to include the rounding parameter taken from context.

javierelpianista · 2024-08-03T22:01:53Z

@skirpichev The DeprecationWarning makes most test cases fail.

To fix this I should modify all calls to to_str to include correct rounding.
This includes the __str__ and __repr__ methods for each type defined in mpmath.
I've made some of these modifications for mpf and mpc types to take the rounding from context. Would you please tell me if you think this is the correct way to go forward?

skirpichev

The DeprecationWarning makes most test cases fail.

It's easy to filter out this warning. My second commit shows how to do this.

To fix this I should modify all calls to to_str to include correct rounding.

Huh, tough decision. It seems that we in any case will introduce a compatibility break. Or - at least - a DeprecationWarning, which might surprise user, as it will come from the mpmath code, that calls to_str().

On another hand, removing legacy code and setting new defaults - leave us with just one test failure in test_str.py. See #832. I'll run diofant's test suite on top of that pr, see diofant/diofant#1430.

I'll appreciate @oscarbenjamin feedback on this.

This includes the str and repr methods for each type defined in mpmath.

Hmm, maybe we won't such flexibility? E.g. gmpy2 uses rounding to nearest, regardless of the context settings. (But see gmpy2/gmpy2#503)

mpmath/libmp/libmpf.py

skirpichev · 2024-08-10T04:25:07Z

Now this has merge conflicts.

skirpichev

This now lacks tests (don't forgot to test the warning too).

But I'm not sure if we should pursue this approach. #832 seems to be working for the Diofant (not test failures). I would appreciate testing on SymPy, however.

Lets wait few days for feedback from SymPy people. If no objections follows - I think we should just switch to round_nearest.

skirpichev

yeah, lest just switch to round_nearest per default

javierelpianista · 2024-08-22T18:23:22Z

yeah, lest just switch to round_nearest per default

OK, so if I understand correctly, I should modify all calls to to_str in the __str__ and __repr__ methods to use round_nearest, correct? Otherwise we would get deprecation warnings for simply printing a float, which is something regular users would not like (myself included)

skirpichev · 2024-08-23T03:46:35Z

OK, so if I understand correctly, I should modify all calls to to_str in the str and repr methods to use round_nearest, correct?

No. Lets just change default in to_str() to round_nearest.

…to tying_support

skirpichev

Again. Please just drop legacy code (rounding=None). Don't test rounding=None.

mpmath/libmp/libmpf.py

javierelpianista · 2024-08-24T19:00:08Z

@skirpichev I think we should assert inside to_str that rounding is one of the correct modes.

skirpichev

we should assert inside to_str that rounding is one of the correct modes.

No. This is a public function and we should raise ValueError.

Please add basic tests for new option.

pyproject.toml

skirpichev · 2024-08-25T07:30:22Z

Hmm, it seems some test(s?) loop forever with your recent changes. Edit: ah, it's new test.

…to tying_support

skirpichev · 2024-08-25T22:56:01Z

I don't think that just removing "bad test" is ok. Why it's failing?

javierelpianista · 2024-08-25T23:48:27Z

I don't think that just removing "bad test" is ok. Why it's failing?

The test I removed aimed to cover line 1237 of libmpf.py. It wasn't relevant for this PR so I removed it without much thought.

skirpichev · 2024-08-26T04:11:17Z

I can reproduce this failure locally with python backend (MPMATH_NOGMPY=1). Maybe it's a normalize() bug.

This reverts commit 38f6e7b.

skirpichev

LGTM, but please update pr title/description

skirpichev · 2024-08-26T06:28:21Z

Thanks.

javierelpianista added 5 commits August 2, 2024 11:00

Included tying support

d1dab6c

* Added new rounding mode: round_nearest_even: round to nearest tying to even. * Modified round_digits to accomodate for this new mode.

Added rounding modes to to_str

c677117

* Wrote abs_rounding function. * Replaced part of the code in to_str with a call to round_digits. * Added tests for different kinds of rounding in test_convert.py

Add tying support for mpf.__format__

37f3b31

* Add new symbol, 'M', for rounding to nearest with tying away from zero. * Replaced part of the code in format_mpf with a call to abs_rounding * Added tests for the M rounding mode.

Add support for round away from zero to mpf.__format__

de76ddd

* Add new symbol, 'Y', for rounding away from zero. * Added tests for the Y rounding mode.

Removed non-flake8 whitespace

88de152

skirpichev self-requested a review August 3, 2024 04:06

skirpichev requested changes Aug 3, 2024

View reviewed changes

mpmath/libmp/libmpf.py Outdated Show resolved Hide resolved

mpmath/libmp/libmpf.py Show resolved Hide resolved

mpmath/libmp/libmpf.py Show resolved Hide resolved

javierelpianista added 5 commits August 3, 2024 16:06

Removed round_nearest_even and all mentions to it

6bf87b2

Modified to_str according to code review suggestions

4d14240

* The default for the rounding parameter is None. * If rounding is None, a DeprecationWarning is raised; the rounding part of the code executes legacy code * Else, the new round_digits function is called

Added rounding argument to tests in test_convert.py

75b91b0

Modified calls to to_str in ctx_mp_python.py

3d31311

* Made all the dunder methods that call to_str to include the rounding parameter obtained from context. This is to avoid the new deprecation warning in to_str.

Modified calls to to_str in libmpc.py

3c10bb7

Modified the call to to_str in mpc_to_str to include the rounding parameter taken from context.

revert everything except libmpf.py

f717d72

skirpichev self-requested a review August 4, 2024 02:50

ignore DeprecationWarning

81e99f1

skirpichev reviewed Aug 4, 2024

View reviewed changes

mpmath/libmp/libmpf.py Outdated Show resolved Hide resolved

javierelpianista mentioned this pull request Aug 4, 2024

Correct formatting for round_up rounding mode #834

Merged

javierelpianista added 2 commits August 10, 2024 09:43

Merge branch 'master' into tying_support

a2c4b76

Move the code in abs_rounding to round_digits

cb63c22

skirpichev reviewed Aug 11, 2024

View reviewed changes

skirpichev requested changes Aug 20, 2024

View reviewed changes

javierelpianista added 4 commits August 24, 2024 10:34

Set to_str default rounding mode to round_nearest

361bb6d

Added tests specific to the old rounding in to_str

08a4446

Restore Deprecation Warning in to_str with rounding=None

01a3a32

Merge branch 'tying_support' of github.com:javierelpianista/mpmath in…

815780b

…to tying_support

skirpichev requested changes Aug 24, 2024

View reviewed changes

mpmath/libmp/libmpf.py Outdated Show resolved Hide resolved

javierelpianista added 3 commits August 24, 2024 15:40

Remove deprecation warning in to_str for rounding=None

7cef7f1

Remove tests for rounding=None in to_str

7ba4d6e

Remove legacy code for rounding=None in to_str

e19cb98

skirpichev requested changes Aug 25, 2024

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

Update pyproject.toml

3693b05

skirpichev and others added 6 commits August 25, 2024 11:29

Merge branch 'master' into tying_support

63449ea

Depend on pytest-timeout

fc174f4

Remove test that causes timeout

a1dd8d9

Raise exception when rounding parameter is incorrect in to_str

89b689c

Add test for wrong rounding in to_str

09314ac

Merge branch 'tying_support' of github.com:javierelpianista/mpmath in…

65733a2

…to tying_support

XXX trying to restore test (no failures locally)

38f6e7b

skirpichev added 2 commits August 26, 2024 07:39

Revert "XXX trying to restore test (no failures locally)"

902224e

This reverts commit 38f6e7b.

+ more tests

02e7d2f

skirpichev requested changes Aug 26, 2024

View reviewed changes

skirpichev changed the title ~~Tying support~~ Support gmpy2-like rounding modes in to_str() Aug 26, 2024

skirpichev approved these changes Aug 26, 2024

View reviewed changes

Merge branch 'master' into tying_support

d7609b2

skirpichev merged commit 7afe9ac into mpmath:master Aug 26, 2024

javierelpianista deleted the tying_support branch August 26, 2024 11:27

skirpichev added this to the 1.4 milestone May 9, 2025

Conversation

javierelpianista commented Aug 2, 2024 • edited by skirpichev Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skirpichev left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

javierelpianista commented Aug 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skirpichev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

skirpichev commented Aug 10, 2024

Uh oh!

skirpichev left a comment

Choose a reason for hiding this comment

Uh oh!

skirpichev left a comment

Choose a reason for hiding this comment

Uh oh!

javierelpianista commented Aug 22, 2024

Uh oh!

skirpichev commented Aug 23, 2024

Uh oh!

skirpichev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

javierelpianista commented Aug 24, 2024

Uh oh!

skirpichev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

skirpichev commented Aug 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skirpichev commented Aug 25, 2024

Uh oh!

javierelpianista commented Aug 25, 2024

Uh oh!

skirpichev commented Aug 26, 2024

Uh oh!

skirpichev left a comment

Choose a reason for hiding this comment

Uh oh!

skirpichev commented Aug 26, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

javierelpianista commented Aug 2, 2024 •

edited by skirpichev

Loading

skirpichev left a comment •

edited

Loading

javierelpianista commented Aug 3, 2024 •

edited

Loading

skirpichev commented Aug 25, 2024 •

edited

Loading