Adds the ability for intword to use user provided mapping. #187
Conversation
… a bug where not conversions would be made if only one mapping is provided.
Codecov Report
@@ Coverage Diff @@
## master #187 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 9 9
Lines 586 592 +6
=========================================
+ Hits 586 592 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| N_("nonillion"), | ||
| N_("decillion"), | ||
| N_("googol"), | ||
| default_conversions = [ |
There was a problem hiding this comment.
Is default_conversions an internal thing? Should we name it _ default_conversions?
There was a problem hiding this comment.
default_conversions is now where powers and human_powers are defined. If powers and human_powers were also internal, then maybe default_conversions should also be internal?
There was a problem hiding this comment.
The existing powers and human_powers are already "public", so we shouldn't change those to _powers and _human_powers in case people are using them.
But do you think people would or should be able to change the default conversions?
There was a problem hiding this comment.
Changing the defaults might not make sense, but people may want to expand them, eg.
from humanize.numbers import intword, default_conversions
intword(1023, conversions=default_conversions.append((3,"Thousands"))But I'm not sure how useful that would be in practice.
There was a problem hiding this comment.
Okay, let's keep it as default_conversions
| @@ -172,16 +178,27 @@ def intword(value, format="%.1f"): | |||
| except (TypeError, ValueError): | |||
| return value | |||
|
|
|||
| if value < powers[0]: | |||
| if conversions is None: | |||
| l_powers, l_human_powers = powers, human_powers | |||
There was a problem hiding this comment.
What does the l_ prefix mean?
There was a problem hiding this comment.
l_ as in local.
I might have done it the other way around, by renaming the default ones, but tests were specifically looking for powers and human_powers in the body of number. I'll change this and adjust the tests as well.
|
|
||
| ``` | ||
| Args: | ||
| value (int, float, str): Integer to convert. | ||
| format (str): To change the number of decimal or general format of the number | ||
| portion. | ||
| conversions [(int, str)]: Power to suffix conversion |
There was a problem hiding this comment.
| conversions [(int, str)]: Power to suffix conversion | |
| conversions [(int, str)]: Power to suffix conversion. |
|
@hugovk thank you for the comments and suggestions. I agree with them, and I'll be changing the PR accordingly. Depending on my personal availability it might take a few days. |
|
🚀 Development has moved to https://github.com/python-humanize/humanize 🚀 Please open new PRs at https://github.com/python-humanize/humanize/pulls |
Adds the ability for intword to use user provided mapping.
Also fixes a bug where not conversions would be made if only one mapping is provided.
I was looking for the ability to humanize into "thousands" in addition to "millions", etc. and noticed it wasn't possible.
This StackOverflow comment lead me to expand intword.
I also saw issues like #160, which lead me to believe I wouldn't be the only one to benefit from this.
Changes proposed in this pull request:
powersandhuman_powersto be defined from a list of pairs(power, human_power)pair is passed tointwordpowersandhuman_powerswithl_powersandl_human_powersto avoid scope conflictsintwordcan receive a list of(power, human_power)pairs when called, to enable custom configurations.If no custom configurations are passed, then it defaults to the existing ones.
For example:
outputs
This lets users expand conversions as needed (eg "thousand") or use custom abbreviations (eg, "k", "MM").