classification
Title: Better document math.copysign behavior.
Type: behavior Stage: resolved
Components: Documentation Versions: Python 3.3, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: docs@python Nosy List: akuchling, docs@python, mark.dickinson, python-dev, r.david.murray, sandro.tosi, terry.reedy, umedoblock
Priority: normal Keywords: patch

Created on 2011-05-29 23:02 by umedoblock, last changed 2014-02-16 17:43 by mark.dickinson. This issue is now closed.

Files
File name Uploaded Description Edit
math_copysign.patch umedoblock, 2011-06-05 02:10 review
math.rst.patch umedoblock, 2011-06-05 03:36 review
issue_12211.patch umedoblock, 2011-06-08 04:06 review
issue_12211_2.patch umedoblock, 2011-06-26 08:52 review
Messages (26)
msg137226 - (view) Author: umedoblock (umedoblock) Date: 2011-05-29 23:02
I expected return int if I gave x as integer to copysign.

I encounterd two problems.
I'd like to fix this problems.
but I can't come up with nice idea.
therefore I just report for you.

one:
>>> import math
>>> a = [0, 1, 2, 3]
>>> i = 1
>>> i_copysign = math.copysign(i, -1)
>>> a[i_copysign]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: list indices must be integers, not float

two:
>>> n = 10 ** 20
>>> math.copysign(n + 1, 1) == n + 1
False
msg137233 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-05-30 02:09
"Except when explicitly noted otherwise, all return values are floats."

On the other hand, copysign says "return x with the sign of y", which certainly sounds like it is preserving x, not creating a new float.  So at the least there is a doc issue, I think.
msg137592 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2011-06-03 21:42
As with all the math docs, 'x' refers to the value, not the object with the value. However, "Return abs(x) with the sign of y" is, to me, clearer and more accurate. Both doc string and doc chapter should get any modification.
msg137628 - (view) Author: umedoblock (umedoblock) Date: 2011-06-04 06:27
abs() behavior show below.
>>> type(abs(-1))
<class 'int'>
>>> type(abs(-1.0))
<class 'float'>

we should fix this problem if write
"Return abs(x) with the sign of y"

I'd like to try this problem if need fix.
I'm going to attach the patch.
msg137641 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2011-06-04 14:43
How about something like: "Return a float with the magnitude of x but the sign of y."?

The behaviour of math.copysign with respect to non-float inputs matches that of almost all the other math module functions:  integer arguments are first converted to floats, and then the underlying libm function applied.  I'm not convinced that changing the behaviour of copysign to produce integer results for integer argument would be a good idea.
msg137669 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2011-06-04 23:35
> "Return a float with the magnitude of x but the sign of y."
This appears to describe both current behavior and what I believe was the intention. I would go with a doc patch based on this.
umedoblock, go ahead and make one.

It occurred to me, also, that as currently written, copysign 'should' return the type of the first arg. In C89, and I suspect in Python 1.0, all math functions return double (Python float). Like Mark, I am more inclined to change the doc than the code.

1. One use of copysign is to give a correctly signed 0.0. This does not apply to ints, where 0 is always unsigned. I do not know of any use of copysign in number (count/int) theory. (There could be, of course.)

2. icopysign(-1,0) == +1 (sign added for emphasis), whereas I believe that for ints, the answer should be -1, as 0 has no sign.

def icopysign(j,k):
    if (j > 0) and (k < 0) or (j < 0) and (k > 0):
        return -j
    return j
for j,k,i in ((1,1,1), (1,-1,-1), (-1,-1,-1), (-1, 1, 1),
              (1,0,1), (-1,0,-1)):
    assert icopysign(j,k) == i, (j,k,i)

This would certainly be up for debate if we changed the code, but there should be no difference in outputs for same value inputs. (This principle is the reason for / and //.) So lets leave copysign as a function defined on floats with inputs coerced to float if needed. Anyone who needs it for ints can define it for (-1,0) according to their need.
msg137673 - (view) Author: umedoblock (umedoblock) Date: 2011-06-05 01:38
I made the patch.
But it cannot pass testCopysign().

math.copysign(1, -0.)
returns 1.

I hope to return -1.
But I don't know how to realize -0. as negative value.

Please help me.
msg137674 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2011-06-05 02:09
umedoblock: David, Mark, and I agree that this should be a doc issue, and so I suggested a DOC patch. So I do not know why you are screwing around with the code, or what you are trying to do with it, or why you are messing around with the version headers or why you think this is a 3.2 only issue.
msg137675 - (view) Author: umedoblock (umedoblock) Date: 2011-06-05 02:10
sorry.
I fix my bug.

but this patch contain new fail...

math.copysign(-1., 0.)
returns 1.
msg137679 - (view) Author: umedoblock (umedoblock) Date: 2011-06-05 03:36
I attached DOC patch.

I misunderstand ?
Sorry...
I think that "go ahead and make one" means I shuold make the source patch.

I use just Python3.2. I didn't use Python 2.x, 3.0 and 3.1 in my programming life.
Therefore I reported version 3.2.
I should the versions to "3rd party" ?
msg137683 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2011-06-05 06:08
Third party refers to things other than Pythonx.y code For instance, distutils2/distribute was for some years developed separately from the main codebase and was recently merged into the 3.3 repository. While separate, its issues were '3rd party'. That has nothing to do with this.

A patch against 3.2 is fine. It will almost certainly apply unchanged to both 3.3 and 2.7 since this part of the doc may have never changed since written. The patch looks fine so far. I see that you kept the line just under 80 characters. Now, can you expand it to also change the docstring for this function in the mathmodule.c file? I am not exactly sure where it is in the file, relative to the function code itself. As I remember, it is not as convenient as in Python files.
It currently looks like

    copysign(x, y)
    
    Return x with the sign of y.

When revising the "Return ..." part to match the doc, I think we should include the "On a platform ..." sentence also. If Mark disagrees, it would be easily removed. Notice that the indent is 1 or 2 more spaces, so the existing line would become too long. 'a platform' could be changed to 'platforms'. I personally like that better anyway.
msg137686 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2011-06-05 08:12
> I think we should include the "On a platform ..."

Sure, sounds good.  One of the main things that makes copysign useful is that it distinguishes between -0.0 and 0.0.
msg137895 - (view) Author: umedoblock (umedoblock) Date: 2011-06-08 04:06
I'm late, sorry.

I attached the patch for math.rst and mathmodule.c.
msg139148 - (view) Author: Sandro Tosi (sandro.tosi) * (Python committer) Date: 2011-06-26 07:49
Taken from http://www.slac.stanford.edu/comp/unix/package/rtems/doc/html/libm/libm.info.copysign.html i'd suggest to extend

  Return a float with the magnitude of x

to

  Return a float with the magnitude (absolute value) of x

It could probably help people less math-savvy in understand what's going to happen :)

Maybe also (only in rest doc) might be nice to describe what happens in case the arguments are NaN, f.e.:

>>> import math
>>> x = float('nan')
>>> math.copysign(1., x)
1.0
>>> math.copysign(-1., x)
1.0
>>> math.copysign(x, -1)
nan
>>> math.copysign(x, x)
nan

umedoblock: would you like to expand the patch with these notes (unless someone objects :)).
msg139154 - (view) Author: umedoblock (umedoblock) Date: 2011-06-26 08:52
sandro: OK, I attached the new patch.
msg139181 - (view) Author: Sandro Tosi (sandro.tosi) * (Python committer) Date: 2011-06-26 14:30
well, what I actually meant is to describe the behavior in case one (or both) of the arguments is NaN (so not cut&pasting the code), while the example was just provided as a quick reference.
msg139191 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2011-06-26 16:54
I agree with adding '(absolute value)'. I think the following covers the NaN behavior. "NaN acts as a positive value that cannot be negated." This should be added to both doc and docstring.

I do not think we generally specify the nan behavior for each function, but it usually follows general rules. The copysign(x,nan) behavior is not obvious as nan, like int 0, does not really have a sign. One might expect copysign(-1.0,nan) to be -1.
msg211322 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-02-16 16:14
New changeset 3ad7725b5013 by Andrew Kuchling in branch '3.3':
#12211: clarify math.copysign() documentation and docstring
http://hg.python.org/cpython/rev/3ad7725b5013
msg211323 - (view) Author: A.M. Kuchling (akuchling) * (Python committer) Date: 2014-02-16 16:16
Applied.  I added two sentences describing the NaN behaviour.
msg211324 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2014-02-16 16:19
The paragraph about NaNs isn't correct.  For example:

>>> from math import copysign
>>> copysign(1.0, float('nan'))
1.0
>>> copysign(1.0, -float('nan'))
-1.0

Though it doesn't really make sense to talk about 'positive' or 'negative' NaNs, a NaN still has a sign bit, and that sign bit is used in the copysign operation.
msg211325 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2014-02-16 16:21
More examples, showing that `y` is not ignored when `x` is a NaN.

>>> result1 = copysign(float('nan'), 1.0)
>>> result2 = copysign(float('nan'), -1.0)
>>> copysign(1.0, result1)  # result1 is a NaN whose sign bit is cleared.
1.0
>>> copysign(1.0, result2)  # result2 is a NaN whose sign bit is set.
-1.0
msg211326 - (view) Author: A.M. Kuchling (akuchling) * (Python committer) Date: 2014-02-16 16:21
OK; I'll just drop the 'If y is NaN' sentence.
msg211327 - (view) Author: A.M. Kuchling (akuchling) * (Python committer) Date: 2014-02-16 16:26
Actually ISTM the 'if x is NaN' sentence can also go; it seems to just copy the sign bit no matter what x and y are.

>>> from math import *
>>> nan = float('nan')
>>> neg = -nan
>>> copysign(+1, copysign(nan, neg))
-1.0
msg211328 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2014-02-16 16:35
> It seems to just copy the sign bit no matter what x and y are.

Yep, that's exactly what happens.  The sign bit from y just gets blindly transferred to x, with no regard to any meaning of x or y.  So it works exactly the same way for zeros, NaNs, infinities, or whatever.
msg211330 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-02-16 17:11
New changeset b01f4ed077fa by Andrew Kuchling in branch '3.3':
#12211: remove paragraph about NaNs
http://hg.python.org/cpython/rev/b01f4ed077fa
msg211332 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2014-02-16 17:43
Thanks!  (And apologies for the nitpicking.)
History
Date User Action Args
2014-02-16 17:43:42mark.dickinsonsetmessages: + msg211332
2014-02-16 17:11:03python-devsetmessages: + msg211330
2014-02-16 16:35:31mark.dickinsonsetmessages: + msg211328
2014-02-16 16:26:15akuchlingsetmessages: + msg211327
2014-02-16 16:21:34akuchlingsetmessages: + msg211326
2014-02-16 16:21:15mark.dickinsonsetmessages: + msg211325
2014-02-16 16:19:04mark.dickinsonsetmessages: + msg211324
2014-02-16 16:16:31akuchlingsetstatus: open -> closed

nosy: + akuchling
messages: + msg211323

resolution: fixed
stage: patch review -> resolved
2014-02-16 16:14:16python-devsetnosy: + python-dev
messages: + msg211322
2013-06-30 06:02:28terry.reedysetversions: + Python 3.4, - Python 3.2
2011-06-26 16:54:18terry.reedysetmessages: + msg139191
2011-06-26 14:30:37sandro.tosisetmessages: + msg139181
2011-06-26 08:52:02umedoblocksetfiles: + issue_12211_2.patch

messages: + msg139154
2011-06-26 07:49:26sandro.tosisetnosy: + sandro.tosi

messages: + msg139148
stage: needs patch -> patch review
2011-06-08 04:06:38umedoblocksetfiles: + issue_12211.patch

messages: + msg137895
2011-06-05 08:12:06mark.dickinsonsetmessages: + msg137686
2011-06-05 06:08:51terry.reedysettitle: math.copysign must keep object type. -> Better document math.copysign behavior.
messages: + msg137683
components: + Documentation, - Library (Lib)
versions: + Python 2.7, Python 3.3
2011-06-05 03:36:45umedoblocksetfiles: + math.rst.patch

messages: + msg137679
2011-06-05 02:10:28umedoblocksetfiles: + math_copysign.patch

messages: + msg137675
2011-06-05 02:09:16terry.reedysetmessages: + msg137674
2011-06-05 02:04:16umedoblocksetfiles: - math_copysign.patch
2011-06-05 01:51:04umedoblocksetfiles: + math_copysign.patch
2011-06-05 01:50:31umedoblocksetfiles: - math_copysign.patch
2011-06-05 01:38:19umedoblocksetfiles: + math_copysign.patch
versions: - Python 2.7, Python 3.3
messages: + msg137673

components: + Library (Lib), - Documentation
keywords: + patch
2011-06-04 23:35:37terry.reedysetassignee: docs@python
components: + Documentation, - Library (Lib)
versions: + Python 2.7, Python 3.3
nosy: + docs@python

messages: + msg137669
stage: needs patch
2011-06-04 14:43:28mark.dickinsonsetmessages: + msg137641
2011-06-04 06:27:58umedoblocksetmessages: + msg137628
2011-06-03 21:42:58terry.reedysetnosy: + terry.reedy
messages: + msg137592
2011-05-30 02:09:53r.david.murraysetnosy: + r.david.murray
messages: + msg137233
2011-05-30 02:07:44r.david.murraysetnosy: + mark.dickinson
2011-05-29 23:02:29umedoblockcreate