Bad sign in sign_mantissa_exponent() methods

[sagetrac-tmonteil]

The .sign_mantissa_exponent() methods of the RealNumber and RealDoubleElement classes give a negative mantissa when the number is negative, which does not corresponds to the behaviour described in the documentation :
http://www.sagemath.org/doc/reference/rings_numerical/sage/rings/real_mpfr.html
http://www.sagemath.org/doc/reference/rings_numerical/sage/rings/real_double.html

We propose here to fix it.

By the way, the variable name 'mantissa' sometimes (always?) appears in the source with the meaning of a signed mantissa. I would suggest to renamed it signed_mantissa or s_mantissa to ease code reading (though i do not feel able to detect which one is signed or not along the code). See for example the .exact_rational() method in real_mpfr.pyx.

---

Apply: attachment: trac_14448_sign_of_mantissa-tm.2.patch

CC: @jasongrout @zimmermann6

Component: basic arithmetic

Keywords: mpfr, RDF

Author: Thierry Monteil

Reviewer: Travis Scrimshaw

Merged: sage-5.10.beta0

Issue created by migration from https://trac.sagemath.org/ticket/14448

[sagetrac-tmonteil]

Attachment: trac_14448_sign_of_mantissa-tm.patch.gz

Tested on sage 5.9.beta5

[sagetrac-tmonteil]

Changed keywords from mpfr, float to mpfr, RDF

[tscrim]

comment:2

I'm guessing this is ready for review?

Looks good, but one small thing, could you change line 2930 in real_mpfr.pyx to:

The mantissa is always a nonnegative number (see :trac:`14448`)::

or something to this extent, noting the problem was fixed in this ticket?

Thanks,

Travis

[jdemeyer]

comment:3

I think it's a bug that the mantissa of 1 and -1 are different.

[sagetrac-tmonteil]

Patch updated so that it quotes this trac ticket

[sagetrac-tmonteil]

comment:5

Attachment: trac_14448_sign_of_mantissa-tm.2.patch.gz

I added the reference to this trac ticket. I also put this ticket under review.

[tscrim]

comment:6

Looks good to me.

[tscrim]

Reviewer: Travis Scrimshaw

[tscrim]

Author: Thierry Monteil

[jdemeyer]

Merged: sage-5.10.beta0