Opened 10 years ago

Closed 7 years ago

#2650 defect closed fixed (fixed)

amp.Integer is either broken or incorrectly documented

Reported by: indigo Owned by:
Priority: low Milestone:
Component: core Keywords: amp, documentation
Cc: Branch:
Author:

Description

"Convert to and from 'int'."

But actually, it converts to and from 'int' and maybe 'long'. If it is intended to not be restricted to probably 32 bit numbers like 'int' (A Good Idea, i think) then this should be explicitly documented, and the documentation should not say 'int'.

Attachments (1)

2650.diff (1.7 KB) - added by zaheerm 7 years ago.
Fixes docstring and adds unit test

Download all attachments as: .zip

Change History (10)

comment:1 Changed 10 years ago by therve

The difference between int and long tends to disappear in Python. I even think long will disappear in Py3K. Some maybe not adding long is a good idea. There is definitely something to be done about the documentation though.

comment:2 Changed 10 years ago by Glyph

Owner: changed from Glyph to indigo

Yeah, the documentation could stand to be improved in more ways than just this. (The implementation is probably a bit too lenient as well; simply int()ing the argument allows for passing strings through and some other probably-not-so-good things.)

comment:3 Changed 10 years ago by Jean-Paul Calderone

Just to be explicit, it is indeed not the intent that Integer be limited to 32 bit values.

comment:4 Changed 10 years ago by Glyph

Priority: normallow

comment:5 Changed 7 years ago by Jean-Paul Calderone

Keywords: documentation added

Changed 7 years ago by zaheerm

Attachment: 2650.diff added

Fixes docstring and adds unit test

comment:6 Changed 7 years ago by zaheerm

Keywords: review added
Owner: indigo deleted

comment:7 Changed 7 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: set to Jean-Paul Calderone
Status: newassigned

Great. The copyright dates on the modified files should be updated, and there needs to be a news file, but I'll take care of that and apply the patch. Thanks.

comment:8 Changed 7 years ago by Jean-Paul Calderone

Resolution: fixed
Status: assignedclosed

(In [28760]) Apply amp.Integer improvement patch

Author: thomasvs, zaheerm Reviewer: exarkun Fixes: #2650

Clarify the API documentation for twisted.protocols.amp.Integer by mentioning that integers of any size are supported and adding an example.

comment:9 Changed 6 years ago by <automation>

Owner: Jean-Paul Calderone deleted
Note: See TracTickets for help on using tickets.