#5689 task closed fixed (fixed)
Remove uses of classical division
Reported by: | Vladimir Perić | Owned by: | Vladimir Perić |
---|---|---|---|
Priority: | normal | Milestone: | Python-3.x |
Component: | core | Keywords: | py3k |
Cc: | Branch: |
branches/classical-division-5689
branch-diff, diff-cov, branch-cov, buildbot |
|
Author: | vperic |
Description (last modified by )
The "classical" division operator "/" has been deprecated for a long while (Python 2.2 I believe) as it is ambiguous - it means floor division with ints, but true division otherwise. In Python 3, the "/" operator is for true division (always returns a float), while the unambiguous operator "" should be used for floor division. As such, it is required to explicitly either import division from __future__
or use the floor division operator.
Most of the time, division was used to force a ZeroDivisionError, for testing purposes. In such cases I've usually imported from __future__
, though in some cases I just changed the line to use the floor operator (as the effect is the same). The only places where floor division actually needed to be used are:
- twisted/conch/connection.py
- twisted/mail/imap4.py
- twisted/mail/smtp.py (date conversion for these two)
- twisted/test/test_protocols.py (can't use floats to call range())
- twisted/web/test/test_webclient.py (ditto)
- twisted/web/test/test_xml.py
- twisted/web/twcgi.py
- twisted/words/test/test_irc.py
Attachments (1)
Change History (10)
Changed 9 years ago by
Attachment: | classic-division.patch added |
---|
comment:1 Changed 9 years ago by
Description: | modified (diff) |
---|
comment:2 Changed 9 years ago by
Keywords: | review removed |
---|---|
Owner: | set to Vladimir Perić |
comment:3 Changed 9 years ago by
Branch: | → branches/classical-division-5689 |
---|
(In [34519]) Create branch classical-division-5689
comment:5 Changed 9 years ago by
BTW, the point of switching to the less ambiguous version is to get rid of the warnings thrown by "-3", so we can notice the actual errors: out of about 120 errors around 20 were real, and this would've been quite hard to notice without fixing everything else. Also, the whole test suite spews out _a lot_ of warnings, making it unwieldy to work with.
I'll write to the list today or tomorrow, and submit the coding standards ticket afterwards (perhaps people will disagree with my suggestions).
Anyway, builds seem to be doing good, so I'll go ahead and merge once the last few Windows bots finish.
comment:6 Changed 9 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
Well, I guess trac didn't catch it correctly, but this is now merged. Closing the ticket manually.
comment:7 Changed 9 years ago by
It's very unfortunate that trac has this problem. The correct way to respond to it is covered on CommitterCheckList though. Please re-read that page to refresh your memory of this and other commit-related tasks you might want to accomplish. Thanks.
comment:8 Changed 9 years ago by
(In [34523]) Merge classical-division-5689: Remove uses of classical division
Author: vperic Reviewer: itamar Fixes: #5689
The "classical" division operator "/" has been deprecated for a long while (Python 2.2 I believe) as it is ambiguous - it means floor division with ints, but true division otherwise. In Python 3, the "/" operator is for true division (always returns a float), while the unambiguous operator "" should be used for floor division. As such, it is required to explicitly either import division from future or use the floor division operator.
Most of the time, division was used to force a ZeroDivisionError, for testing purposes. In such cases, I've usually imported from future, though in some cases I just changed the line to use the floor operator. These changes are still important, as they will hide the warnings produced by "-3" and make it easier to spot the real errors (of which there were about twenty).
comment:9 Changed 9 years ago by
Hi exarkun, thanks for fixing this. I did follow the steps, I checked the post-commit log but couldn't find any errors reported, so I brought it up on IRC and itamar told me to just close it manually. I guess I just missed the relevant line, I'll be more careful if it happens again.
I'm not sure what the point of switching to a less ambiguous version of '1/0' is, since it will do ZeroDivisionError regardless. But I guess the coding standard is going to require non-ambiguity, so I suppose we should do this.
So, looks good - do the usual buildbot run, then merge if it passes (and don't forget to add this to the coding standard ticket).
Also - you might want to mention this change on the mailing list, so people are on the lookout for
from __future__ import division
. You might want to mention *all* Python 3 coding standard changes, but this one in particular impacts the behavior of whole modules. An argument could be made that we should *require* it on all twisted modules, much like we want all new classes to be new style, so you should bring that up.