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

[en] improve EN_A_VS_AN #10358

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AzadehSafakish
Copy link
Collaborator

@AzadehSafakish AzadehSafakish commented Feb 29, 2024

If "a" or "an" is followed by "EUR", look to the next token to determine the correct determiner.

I needed to add "100" to det_a.txt to make the 3rd example work:
assertIncorrect("The company has also entered into an EUR 100 million ($88 million) debt");

Not sure why I had to do this, but I do notice that:

The company has also entered into an 100 million USD debt

is not caught by this particular rule either.
Maybe there's a blind spot with certain numerical values.

@@ -95,6 +95,11 @@ public RuleMatch[] match(AnalyzedSentence sentence) {
}
if (equalsA || equalsAn) {
Determiner determiner = getCorrectDeterminerFor(token);
if (token.getToken().equals("EUR") && i < tokens.length - 1) {
determiner = getCorrectDeterminerFor(tokens[i + 1]);
} else if (token.getToken().equals("EUR") && i == tokens.length - 1) {
Copy link
Member

Choose a reason for hiding this comment

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

There's no test for this case, or am I missing something?

@@ -95,6 +95,11 @@ public RuleMatch[] match(AnalyzedSentence sentence) {
}
if (equalsA || equalsAn) {
Determiner determiner = getCorrectDeterminerFor(token);
if (token.getToken().equals("EUR") && i < tokens.length - 1) {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a comment with an example here, as this is quite clever but maybe a bit difficult to understand by just looking at the code?

@AzadehSafakish AzadehSafakish marked this pull request as draft March 4, 2024 12:07
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.

2 participants