-
-
Notifications
You must be signed in to change notification settings - Fork 10.9k
difference between behavior of assignment division and future division not documented (Trac #2061) #2653
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
Comments
@rgommers wrote on 2012-02-21 There is a big red warning box in the place I'd expect: http://docs.scipy.org/doc/numpy/reference/arrays.ndarray.html#arithmetic-and-comparison-operations |
@mwiebe wrote on 2012-02-22 To add my opinion here: I want this case to raise an exception by default based on the 'same_kind' casting rule. e.g.
|
trac user AlanFrankel wrote on 2012-02-22 In response to mwiebe: Would the behavior (generation of an error) differ depending on whether the division operator had been imported from future? In response to rgommers: The red box is not where I expected it, is not very prominent on the page, and does not seem particularly relevant to the issue at hand at first glance. Nowhere on that page do the strings "+=", "*=, "/=" appear, for instance. In fact, when I search for "/=" from the NumPy search box, no documentation at all comes up. The connection between the double-underscore-decorated methods (e.g., ndarray.truediv) and the assignment operators is not obvious, and the fact that there are at least two issues at play (type coercion and truediv/div alternation) only makes the situation more confusing, especially in the absence of explanation. I won't have the time to come back to this immediately, but I am reopening it because I think the documentation in this area should be revisited. |
@rgommers wrote on 2012-02-22 Mark, |
@rgommers wrote on 2012-02-22 Alan, I don't have a problem with adding the explicit strings "/=", "+=", etc. to that section of the documentation if that would help. A patch on the doc wiki, github or here is welcome. As to the location and visibility, we're just going to have to agree to disagree then. Here's my reasoning:
IMHO this ticket can be closed again, but I'll leave it to someone else to do so. |
@mwiebe wrote on 2012-02-22 With 'same_kind', there would be a slight difference, a consequence of how the types work. Python 2 has int / int -> int, so array(int) /= array(int) is ok. Python 3 has int / int -> float, so array(int) /= array(int) is trying to assign a float into an int array, and would raise an error. |
@charris wrote on 2012-02-22 Already ran into the python 3 error in the ndimage tests, I assume it is enabled in current master. |
@charris wrote on 2012-02-22 I think we can leave the ticket open for a bit in case Alan submits a patch. Alan, it is generally considered rude to reopen a closed ticket, so you should probably apologize in advance when doing so ;) The mailing list is another good place to look for for a jury trial rather than summary judgment. |
@mwiebe wrote on 2012-02-22 Yes, my memory failed me on this one. This was the particular case I had argued for: http://mail.scipy.org/pipermail/numpy-discussion/2011-June/056605.html This is just a small step towards tighter rules that protect programmers from "silly" errors. I would still argue in favour of keeping the change in, and adding information about it to the release notes. Clearly this should have gone in the release notes, with a special remark to trigger discussion before the release, but I didn't think of it, and the code review didn't flag it as missing either. Probably "The Checklist Manifesto" applies to situations like these. With Travis now more actively involved, I'm certain he would like the opportunity to examine the changes and weigh in. |
@charris wrote on 2012-02-22 You must have missed one of the ndimage fixes, or else new tests were added, since another just went in ;) I agree that tighter conversion rules are the way to go. There is a tradeoff between convenience and correctness, but in this case I think explicit use of '//' makes sense and leads to clearer code and fewer inexplicable results. |
@mwiebe wrote on 2012-02-23 Looks like I was wrong again, and actually I did put it in the release notes. Here's what it says: The default casting rule for UFunc out= parameters has been changed from 'unsafe' to 'same_kind'. Most usages which violate the 'same_kind' rule are likely bugs, so this change may expose previously undetected errors in projects that depend on NumPy. |
@teoliphant wrote on 2012-02-23 I can support the "same_kind" casting approach for 2.0 and as a context-sensitive parameter in the 1.X series if we choose to spend the time on it. setitem will need the same work. The current behavior can be confusing at first, but it is consistent with the "truncating" behavior of other things in NumPy (like setitem). There is an important difference between a = value and a = a value operation. The later allows for a complete change in the object that 'a' is bound to, while the former does not. Going to "same kind" can still make clear this difference but perhaps remove the most glaring surprises. |
Original ticket http://projects.scipy.org/numpy/ticket/2061 on 2012-02-21 by trac user AlanFrankel, assigned to @pv.
I just ran into the same issue described in bug 1734 (assignment division not compatible with future division). Namely, I expected to see that the results of
array_a /= scalar_s
would depend on whether a "from future import division" statement was included in the code. Eventually, I figured out this had no effect; the only way I could get true division, which I wanted, was to use:
array_a = array_a / scalar_s
I see that bug 1734 has been marked "won't fix". Whether or not I agree with that assessment, I definitely think that this behavior should be in user-accessible documentation (not just in the bug database). The "Mathematical Functions" page ( http://docs.scipy.org/doc/numpy/reference/routines.math.html ) doesn't have any reference to the assignment operators at all (+=, -=, *=, /=), which in my opinion is a glaring omission, especially since it's hard to search for these strings via a punctuation-stripping search engine. The assignment operators should be listed (or there should be a link to them), and the fact that "/=" gives different results from "/" should be stated.
The text was updated successfully, but these errors were encountered: