Skip to content
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

Support ordinals #6

Open
noviluni opened this issue Jun 17, 2020 · 3 comments
Open

Support ordinals #6

noviluni opened this issue Jun 17, 2020 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@noviluni
Copy link
Contributor

I open this ticket to track the ordinal's feature.

From my understanding, what we should achieve is:

>>> parse('first')
'1st'

>>> parse('second')
'2nd'

>>> parse('third')
'3rd'

>>> parse('twenty-third')
'23rd'

>>> parse('thirtieth')
'30th'

However, as we support other words in the sentence, we should probably take care of some ambiguous words. I would take special care to "second". I think it should be translated to "2nd" only when it's not preceded by:

  • 1 (example: "1 second")
  • one (example: "one second")
  • a (example "a second").
  • another ordinal (examples: "first second" --> "1st second" or "fourth second" --> "4th second").

Of course, this logic would be probably necessary to be applied only to some languages, so it shouldn't be inside the main logic but in a language-specific section.

@arnavkapoor arnavkapoor self-assigned this Jun 17, 2020
@arnavkapoor arnavkapoor added the enhancement New feature or request label Jun 17, 2020
@arnavkapoor
Copy link
Collaborator

Hi @noviluni so I had begin working on the support for ordinal numbers. The best approach I believe is to create similar structure like the cardinal numbers. One direction was to somehow extend the cardinal numbers to handle ordinal too. (storing additional suffix only , example th for English ). However there is a major difference between the ordinal and cardinal number in other languages.

22 - veintidós
22nd - vigésimo segundo

So, thus I plan to update the data files with the following proposed structure. I am thinking of adding the tokens for negative and decimal numbers too for future features. (For English negative_tokens might be 'minus', 'negative' and decimal_tokens would be 'point', 'dot' )

{
    "CARDINAL_NUMBERS": {
        "UNIT_NUMBERS": {},
        "DIRECT_NUMBERS": {},
        "TENS": {},
        "HUNDREDS": {},
        "BIG_POWERS_OF_TEN": {}
    },
    "ORDINAL_NUMBERS":{
        "UNIT_NUMBERS": {},
        "DIRECT_NUMBERS": {},
        "TENS": {},
        "HUNDREDS": {},
        "BIG_POWERS_OF_TEN": {}
    },
    "SKIP_TOKENS": [],
    "NEGATIVE_TOKENS": [],
    "DECIMAL_TOKENS":[],
    "LONG_SCALE": false
}

@noviluni
Copy link
Contributor Author

noviluni commented Aug 3, 2020

Hi @arnavkapoor! It looks good! However, I'm not 100% sure of adding negative and decimal tokens right now for two reasons:

  • When trying to implement it, it could be possible that we find other aspects we don't consider right now and this approach wouldn't be working.
  • Every time we release a package, we usually expect to have a "stable" code. Adding this without being used, could cause confusion to users/devs, and make them crazy. We could wait to implement this (negative and decimal numbers) before releasing a new version, but we will probably need to release the new version with ordinal numbers support before doing it.

Does this make sense?

About the naming, it's ok :). Maybe we could change CARDINAL_NUMBERS by just NUMBERS, but up to you.

@arnavkapoor
Copy link
Collaborator

arnavkapoor commented Aug 26, 2020

Currently ordinal number support exists for only English language. #31 (review) . There needs to be changes to incorporate other languages. One way could be updating the _apply_cardinal_conversion mentioned here for other languages #31 (comment) .
The other could be creating same structure as cardinal number for ordinal number.
The merged PR for ordinal number support for English is #35

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants