Opened 17 years ago

Closed 17 years ago

Last modified 17 years ago

#3017 closed enhancement (fixed)

[patch] unit tests showing humanize intcomma handling floats

Reported by: Chris Beaven Owned by: Adrian Holovaty
Component: Contrib apps Version:
Severity: normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The intcomma filter should handle floats, too.

Of course, the name doesn't make much sense in that case, so I have renamed to commaseparate (while keeping backwards compatibility).

Attachments (3)

commaseparate.patch (1.9 KB ) - added by Chris Beaven 17 years ago.
commaseparate.2.patch (2.1 KB ) - added by Chris Beaven 17 years ago.
humanize.diff (894 bytes ) - added by Russell Keith-Magee 17 years ago.
unit test for humanize.intcomma

Download all attachments as: .zip

Change History (11)

by Chris Beaven, 17 years ago

Attachment: commaseparate.patch added

comment:1 by Chris Beaven, 17 years ago

On a related side note, there's one spelling mistake in a docstring somewhere which misspells separate as "seperate".

I can't be bothered opening a ticket for something that negligible, but feel free to fix it as part of this one ;)

by Chris Beaven, 17 years ago

Attachment: commaseparate.2.patch added

comment:2 by Chris Beaven, 17 years ago

Triage Stage: UnreviewedReady for checkin

comment:3 by Malcolm Tredinnick, 17 years ago

I'm going to leave this one until after 0.96 (which will be "real soon now"), just to avoid any confusion with the renaming change (I realise this is backwards compatible; good thinking there). Between 0.96 and 1.0, the list of things people will have to look out for will grow and we'll include this in that.

by Russell Keith-Magee, 17 years ago

Attachment: humanize.diff added

unit test for humanize.intcomma

comment:4 by Russell Keith-Magee, 17 years ago

Triage Stage: Ready for checkinDesign decision needed

Are you sure this change is required? I added a unit test for float separation (diff attached), and it worked fine as-is.

Can you provide a specific test case where the existing intcomma filter fails for floats?

comment:5 by Chris Beaven, 17 years ago

Resolution: worksforme
Status: newclosed

I'm quite confused also - I can see the current method works perfectly with floats so I'm not sure why I even wrote this patch.

comment:6 by Chris Beaven, 17 years ago

Resolution: worksforme
Status: closedreopened
Summary: [patch] Humanize intcomma should handle floats[patch] unit tests showing humanize intcomma handling floats
Triage Stage: Design decision neededReady for checkin

Actually, the unit test may as well go in there. I opened a new ticket regarding renaming the filter to something more sane: #3884

comment:7 by Malcolm Tredinnick, 17 years ago

Resolution: fixed
Status: reopenedclosed

(In [4880]) Fixed #3017 -- Added tests for humanize filter.

comment:8 by Malcolm Tredinnick, 17 years ago

Russell: I changed one of the tests (the one with a long float) before committing, because it was vulnerable to internal floating-point representation problems. I only caught it because it failed for me.

Note: See TracTickets for help on using tickets.
Back to Top