Skip to content

Conversation

@nciric
Copy link
Contributor

@nciric nciric commented Sep 10, 2025

Don't require full lexical info about the noun (gender and number) when inflecting with rules. Update tests to reflect that.

Part of issue #172

@nciric nciric requested a review from grhoten September 10, 2025 16:21
Copy link
Member

@grhoten grhoten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK, but you can go further and remove the pos. Case is the most important part to inflect normally.

<test><source case="vocative" pos="noun">Стана</source><result>Стано</result></test>
<test><source case="vocative" pos="noun">Зора</source><result>Зоро</result></test>
<test><source case="vocative" pos="noun">Божа</source><result>Божо</result></test>
<test><source case="vocative" pos="noun">Љуба</source><result>Љубо</result></test>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also default to noun. Every other language assumes noun by default. Every other part of speech is an exception. That's important for quantities.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you default to POS_NOUN? I looked around other languages but couldn't find an obvious solution in the codebase.

Copy link
Member

@grhoten grhoten Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you try just removing it?

Your dictionary inflector has the following. It means prioritize nouns over adjectives when there is ambiguity.

    , dictionaryInflector(::inflection::util::LocaleUtils::SERBIAN(),{
            {GrammemeConstants::POS_NOUN(), GrammemeConstants::POS_ADJECTIVE()},

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did and it just works - so the system will take nouns first, then adjectives as that's the order in the dictionaryInflector. Good to know.

@nciric nciric merged commit 500bcd2 into main Sep 11, 2025
8 checks passed
@nciric nciric deleted the cira-sr branch September 11, 2025 11:32
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