#20122 closed Cleanup/optimization (fixed)
Pluralize filter sometimes returns singular form instead of an empty string for invalid inputs
Reported by: | Aymeric Augustin | Owned by: | Tobias Kunze |
---|---|---|---|
Component: | Template system | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Jorge C. Leitão | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Filters are documented to return either their input unchanged, or the empty string, whatever makes most sense, when they're used incorrectly. The pluralize
filter returns the empty string in such cases, for instance when it receives more than two forms (singular, plural).
However, it returns the singular form instead of the empty string when it's passed an object that isn't a number, a string or a list.
Failing test case:
--- a/tests/defaultfilters/tests.py +++ b/tests/defaultfilters/tests.py @@ -622,6 +622,9 @@ class DefaultFiltersTests(TestCase): self.assertEqual(pluralize(2,'y,ies'), 'ies') self.assertEqual(pluralize(0,'y,ies,error'), '') + def test_pluralize_error(self): + self.assertEqual(pluralize(object, 'y,ies'), '') + def test_phone2numeric(self): self.assertEqual(phone2numeric_filter('0800 flowers'), '0800 3569377')
I understand that the implementation is crafted to avoid isinstance
checks, but in this case we really want different logic depending on the type of the input. I think the filter should be rewritten with the following pseudo-code:
if the value is a number: return singular if value is 1 else plural if the value is a string: return singular if value is '1' else plural if the value has a length (needs a try/except TypeError): return singular if length is 1 else plural return ''
I discovered this while working on #16723.
Attachments (1)
Change History (12)
comment:1 by , 12 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 11 years ago
comment:3 by , 11 years ago
Cc: | added |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:4 by , 11 years ago
The added patch provides the following functionality:
if value is 1 or '1' returns singular elif it is a string representable by a float returns plural elif is something that can be argument of len(): if len(value) == 1: returns singular else: returns plural else, returns ''
This complements the existing functionality, extends to float numbers, and fixes this ticket, returning in some case where a number (plural/singular) cannot be defined by the value.
by , 11 years ago
Attachment: | defaultfilters_fix_pluralize.diff added |
---|
comment:5 by , 11 years ago
Has patch: | set |
---|
comment:6 by , 11 years ago
Needs tests: | set |
---|---|
Patch needs improvement: | set |
comment:7 by , 6 years ago
Needs tests: | unset |
---|---|
Owner: | changed from | to
comment:8 by , 6 years ago
Patch needs improvement: | unset |
---|
I don't agree with the suggested implementation because when you pass a string to a pluralize it should not assume the string can be "pluralized". In your case, if you pass 'one', it would return the plural, and I think that it should return an empty string: it is language dependent whether strings represent singular or plural number.
One possible solution that compromises both cases:
This avoids the
isinstance()
and also corrects the pluralization of floats (1 unit but 1.1 units).Cheers,
Jorge