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

Add gender and morphological cases support for Ukrainian #530

Merged
merged 4 commits into from
Sep 6, 2023

Conversation

kant2002
Copy link
Contributor

Added two parameters: gender and case
gender can accept either 'masculine' (default) or 'feminine' case can accept either 'nominative' (default) or
'genitive','dative','accusative','instrumetnal','locative' and 'vocative'.

This parameters now working only for to='cardinal'

Changes proposed in this pull request:

  • Add ability optionally specify gender when convert to text
  • Add ability optionally specify case when convert to text

Status

  • READY
  • HOLD
  • WIP (Work-In-Progress)

How to verify this change

Personally I use this one website https://vshkole.in.ua/vidminiuvannia-chislivnikiv/

I can lookup at more interesting English sources, but it will take time. Also I will try ask for help from local NLP community if that's helpful.

Additional notes

Slavik languages can have varying forms for words based on grammatic case and gender, so if attempt to convert numbers embedded in the sentence accounting for the form is required. I assume that it's responsibility of the caller to properly identify case and gender of related words.

Added two parameters: gender and case
gender can accept either 'masculine' (default)  or 'feminine'
case can accept either 'nominative' (default) or
'genitive','dative','accusative','instrumetnal','locative' and
'vocative'.

This parameters now working only for to='cardinal'
@kant2002
Copy link
Contributor Author

@mrodriguezg1991 can I bring your attention to this PR? Do you think this is valuable addition?

@kant2002
Copy link
Contributor Author

I reformat source code

@kant2002
Copy link
Contributor Author

I reformat tests. Did not realize that I should check them, I cannot replicate setup locally, since I use venv and it create false positives for me somehow.

@coveralls
Copy link

Coverage Status

coverage: 97.397% (-0.002%) from 97.399% when pulling 6c8e66b on kant2002:kant/add-cases into 872510d on savoirfairelinux:master.

@kant2002
Copy link
Contributor Author

What's my next steps? I see coverage setting go down only in Vietnamese which I did not touch. Is this some infra problem?

@mrodriguezg1991 mrodriguezg1991 merged commit 3e39091 into savoirfairelinux:master Sep 6, 2023
6 checks passed
@kant2002 kant2002 deleted the kant/add-cases branch September 9, 2023 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants