[Django] #20122: Pluralize filter sometimes returns singular form instead of an empty string for invalid inputs
Django
for invalid inputs
------------------------------------------------+------------------------
Reporter: aaugustin | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Template system | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/20122>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
for invalid inputs
Reporter: aaugustin | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Template system | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/20122#comment:1>
Django
for invalid inputs
Reporter: aaugustin | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Template system | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by littlepig):
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:
{{{
if value == 1 or value == u'1':
return singular
try:
float(value)
return plural
except ValueError:
# a string that cannot be parsed as a float and thus is an undefined
number
return ''
}}}
This avoids the {{{isinstance()}}} and also corrects the pluralization of
floats (1 unit but 1.1 units).
Cheers,
Jorge
--
Ticket URL: <https://code.djangoproject.com/ticket/20122#comment:2>
Django
for invalid inputs
Reporter: aaugustin | Owner: littlepig
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Template system | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by littlepig):
* owner: nobody => littlepig
* status: new => assigned
* cc: littlepig (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/20122#comment:3>
Django
for invalid inputs
Reporter: aaugustin | Owner: littlepig
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Template system | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by littlepig):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/20122#comment:4>
Django
for invalid inputs
Reporter: aaugustin | Owner: littlepig
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Template system | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/20122#comment:5>
Django
for invalid inputs
Reporter: aaugustin | Owner: littlepig
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Template system | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by lrekucki):
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/20122#comment:6>
Django
for invalid inputs
Reporter: Aymeric Augustin | Owner: Tobias
Type: | Kunze
Cleanup/optimization | Status: assigned
Component: Template system | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Tobias Kunze):
* owner: Jorge C. Leitão => Tobias Kunze
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/20122#comment:7>
Django
for invalid inputs
Reporter: Aymeric Augustin | Owner: Tobias
Type: | Kunze
Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tobias Kunze):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/20122#comment:8>
Django
for invalid inputs
Reporter: Aymeric Augustin | Owner: Tobias
Type: | Kunze
Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"e3968df527c4d378677f4784fb1bc0c86950fcf8" e3968df5]:
{{{
#!CommitTicketReference repository=""
revision="e3968df527c4d378677f4784fb1bc0c86950fcf8"
Refs #20122 -- Corrected documentation of pluralize template filter.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20122#comment:9>
Django
for invalid inputs
Reporter: Aymeric Augustin | Owner: Tobias
Type: | Kunze
Component: Template system | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"e68361364906170db936d917ab5dd07262f537b6" e6836136]:
{{{
#!CommitTicketReference repository=""
revision="e68361364906170db936d917ab5dd07262f537b6"
Fixed #20122 -- Made pluralize template filter return '' on invalid input.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20122#comment:10>
Django
for invalid inputs
Reporter: Aymeric Augustin | Owner: Tobias
Type: | Kunze
Cleanup/optimization | Status: closed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"bf9e0e342da3ed2f74ee0ec34e75bdcbedde40a9" bf9e0e34]:
{{{
#!CommitTicketReference repository=""
revision="bf9e0e342da3ed2f74ee0ec34e75bdcbedde40a9"
[2.2.x] Refs #20122 -- Corrected documentation of pluralize template
filter.
Backport of e3968df527c4d378677f4784fb1bc0c86950fcf8 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20122#comment:11>