Better document math.copysign behavior.

[umedoblock]

BPO	12211
Nosy	@akuchling, @terryjreedy, @mdickinson, @bitdancer, @sandrotosi
Files	math_copysign.patch math.rst.patch issue_12211.patch issue_12211_2.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2014-02-16.16:16:31.413>
created_at = <Date 2011-05-29.23:02:29.970>
labels = ['type-bug', 'docs']
title = 'Better document math.copysign behavior.'
updated_at = <Date 2014-02-16.17:43:42.843>
user = 'https://bugs.python.org/umedoblock'

bugs.python.org fields:

activity = <Date 2014-02-16.17:43:42.843>
actor = 'mark.dickinson'
assignee = 'docs@python'
closed = True
closed_date = <Date 2014-02-16.16:16:31.413>
closer = 'akuchling'
components = ['Documentation']
creation = <Date 2011-05-29.23:02:29.970>
creator = 'umedoblock'
dependencies = []
files = ['22251', '22254', '22278', '22480']
hgrepos = []
issue_num = 12211
keywords = ['patch']
message_count = 26.0
messages = ['137226', '137233', '137592', '137628', '137641', '137669', '137673', '137674', '137675', '137679', '137683', '137686', '137895', '139148', '139154', '139181', '139191', '211322', '211323', '211324', '211325', '211326', '211327', '211328', '211330', '211332']
nosy_count = 8.0
nosy_names = ['akuchling', 'terry.reedy', 'mark.dickinson', 'r.david.murray', 'sandro.tosi', 'docs@python', 'python-dev', 'umedoblock']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue12211'
versions = ['Python 2.7', 'Python 3.3', 'Python 3.4']

[umedoblock]

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

[bitdancer]

"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.

[terryjreedy]

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.

[umedoblock]

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.

[mdickinson]

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.

[terryjreedy]

"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.

[umedoblock]

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.

[terryjreedy]

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.

[umedoblock]

sorry.
I fix my bug.

but this patch contain new fail...

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

[umedoblock]

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" ?

[terryjreedy]

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.

[mdickinson]

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.

[umedoblock]

I'm late, sorry.

I attached the patch for math.rst and mathmodule.c.

[sandrotosi]

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 :)).

[umedoblock]

sandro: OK, I attached the new patch.

[sandrotosi]

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.

[terryjreedy]

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.

[python-dev]

New changeset 3ad7725b5013 by Andrew Kuchling in branch '3.3':
bpo-12211: clarify math.copysign() documentation and docstring
http://hg.python.org/cpython/rev/3ad7725b5013

[akuchling]

Applied. I added two sentences describing the NaN behaviour.

[mdickinson]

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.

[mdickinson]

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

[akuchling]

OK; I'll just drop the 'If y is NaN' sentence.

[akuchling]

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

[mdickinson]

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.

[python-dev]

New changeset b01f4ed077fa by Andrew Kuchling in branch '3.3':
bpo-12211: remove paragraph about NaNs
http://hg.python.org/cpython/rev/b01f4ed077fa

[mdickinson]

Thanks! (And apologies for the nitpicking.)