-
Notifications
You must be signed in to change notification settings - Fork 71
Have ordinal return 0th instead of 0 #80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Thank you for the careful thought and consideration as well as your time and effort into this pull request. Just because a word is defined does not make it colloquial. As a native speaker I cannot think of a case where zeroth makes sense in conversation. Anything in the place of zeroth, to my mind, spoken still seems like it should be "first". Which we cannot guarantee is correct. I'll continue looking for an example where this is correct until tomorrow. Unless we think of an example of a reasonable use case I'm closing this. Thanks again for your thought and time. |
|
No worries, I was using this to ordinalize percentiles from postgres' 0th or zeroth are mostly in use in CompSci and Mathematics, as far as I can tell, so I might be suffering from a 'slight' bias here ;). Some examples from those fields:
The last one of those even says as much:
Anyhoo, I completely defer to you here, since I no longer have a horse in the race here. |
|
Thanks! dI'l be happy to revisit and have a conversation it it covers up again. |
|
I think this use case makes sense in those context. My suggestion would be to to extend ordinal(value, options = {}) {
const { scientific = false } = options; // Maybe a better name...
const number = parseInt(value, 10);
if (number === 0 && !scientific) {
return value;
}
...
}This behavior should definitely be opt-in though and by default should use the current humanization. |
|
+1 |
|
I added a currently adding docs for the option |
| }); | ||
|
|
||
| it('should return the input if the number is 0', () => { | ||
| expect(Humanize.ordinal('0.')).toEqual('0.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if the method should be updated to return the string version of the parsed input rather than the original input.
expect(Humanize.ordinal(0)).toEqual('0');
// and
expect(Humanize.ordinal('0.foobar')).toEqual('0');There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would make sense to me, since ordinal(1.foo) returns 1st, not 1.foost. It would however be somewhat of an backwards incompatible change, so I'll shoot in a separate PR for that
Both dictionary.com and wikipedia recognize 0th or zeroth, which makes this special case a bit weird to me