Intword and intabr expansion and intword_internal api exposure

[James Reynolds]

Created a pull requests located here: ​https://github.com/django/django/pull/578

This patch does three things:

1. Changes internal API for intword and adds an optional argument for precision. Default is 1, which was previous hardcoded behavior.
2. Adds a new filter called "intabr" that does the same as the above, except returns the converted number with an abbreviation.
3. Exposes an API for developers to create there own word conversion type filters without needing to recreate the wheel.

Hopefully I didn't screw up the pull request process - this is the first one I've attempted.

[Russell Keith-Magee]

It took me a while to work out what you were trying to do here. Suggestion for the future -- when proposing a feature, don't bury the lede. The suggestion here is to add two features:

• an intabr template tag that returns 1.2M rather than 1.2 million, and
• a precision option for the intword/abr flag.

Changes to internal API and implementation are largely irrelevant to the end effect. They might be worth mentioning as a way of validating why your patch implements a feature a particular way.

Accepting the broad concept. However, regarding implementation:

• I don't accept the need for intword_internal. What's the use case?
• The implementation seems excessively complex. What you're doing is a very simple lookup of prefixes based on the number of decimal places. Why does this require an interable class instance, other than to prove that you know how to use Python iterators?
• I'm not wild about intabr as a name for the new template tag. If we were going to abbreviate at all, I'd use abbr, not abr; but I'd prefer a better name altogether. Suggestions welcome, but inthuman and intlarge are two to start with.

[James Reynolds]

This is my first real attempt at this, so I appreciate the feedback.

Specifically with respect to the points you raised:

• I don't accept the need for intword_internal. What's the use case?

The idea was to not only provide some commonly used filters, but also allow for this situation to be easily modified. I often have to abbreviate numbers and the treatment is not always consistent, resulting in a custom filter. This function allows users to create custom filters with a single line of code. True, they could call _intword, but in case something should change, intword_internal should remain consistent.

• The implementation seems excessively complex. What you're doing is a very simple lookup of prefixes based on the number of decimal places. Why does this require an interable class instance, other than to prove that you know how to use Python iterators?

Related to the above, the idea was to expose a simple API, and this includes allowing users to provide their own iterables on an as needed basis. I could have iterated over the list itself in the _intword function, but my preference was and still is to decouple the Converter class from the filter itself and allow users to create arbitrary iterators, provided they accept the arguments setforth in the function.

• I'm not wild about intabr as a name for the new template tag. If we were going to abbreviate at all, I'd use abbr, not abr; but I'd prefer a better name altogether. Suggestions welcome, but inthuman and intlarge are two to start with.

I have no issues with changing the filter name to intabbr. I would prefer not to change to inthuman or intlarge. This is simply because of the older filter intword and I want the naming convention to be consistent.

Please let me know of any modifications that I should take, and I will make them as soon as possible. I look forward to hearing back from you.

[James Reynolds]

Replying to russellm:

Hey Russell:

Just wondering if you've had a change to take a look at my comments to your earlier questions?

[Luke Plant]

Our policy for all APIs is that they must be documented, or they are considered internal implementation details - it's all or nothing. At the moment, your 'Converter' class lies half-way - its existence is documented, but not its methods. "Just inherit from Converter" is not a documented API.

On the other hand, documenting it seems like overkill - none of the other filters have ways of generating similar filters.

The bar for inclusion in Django is high - it needs to fulfil a common need, and generating your own filters for displaying numbers does not seem to be that common. If we are not going to fully document the API for creating your own filters, then there is no point having that layer of complication.

To me, at the moment, it seems that this would work just as well as an external library, where you are free to design and document it as you want.

[Aymeric Augustin]

I agree with Luke, we already have many built-in filters and we're not particularly looking at adding more.