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

[pt] disambiguation.xml fix #10861

Merged
merged 4 commits into from
Sep 3, 2024
Merged

Conversation

marcoagpinto
Copy link
Member

Heya, @susanaboatto , @p-goulart and @jaumeortola ,

I have fixed rare verbs detecting:

Quero duas garotas.
Quero duas partes.

"garotas" and "partes" no longer appear as verbs.

But it will impact in the whole core of PT.

That is why I am always scared to change the disambiguator.

But if you believe it is okay, then it is a major improvement, and it will fix tons of false positives in our rules.

@marcoagpinto
Copy link
Member Author

Ahhhh... it now also fixes:

Quero as duas militares.
Quero os dois militares.

"militares" no longer appears as a verb.

@marcoagpinto
Copy link
Member Author

Sorry!!!!!!!

I have just fixed it:
Para as duas matares.

If the words are not also nouns, it won't remove them as verbs.

@marcoagpinto
Copy link
Member Author

Now it is working again with:

Quero duas garotas.
Quero duas partes.
Quero as duas militares.
Quero os dois militares.
Para as duas matares.

Copy link
Collaborator

@p-goulart p-goulart left a comment

Choose a reason for hiding this comment

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

Seems okay to me.

@marcoagpinto
Copy link
Member Author

@p-goulart

Pedro, I am so stressed!

😛 😛 😛 😛 😛 😛 😛

Thanks!

@marcoagpinto marcoagpinto merged commit b52fbb5 into master Sep 3, 2024
5 checks passed
@marcoagpinto marcoagpinto deleted the lt_marcoagpinto_20240903_0613 branch September 3, 2024 07:12
@p-goulart
Copy link
Collaborator

The rule seems tightly scoped, I doubt this will have many undesired side effects. But do keep an eye on the diff tomorrow morning.

@susanaboatto probably wiser not to include this in a premium deployment, if it does happen later today (I know there may be some changes to the schedule, but just in case.)

@marcoagpinto
Copy link
Member Author

Yes, at 5am I will look at the diff to check if some exceptions need to be added.

@marcoagpinto
Copy link
Member Author

@p-goulart @susanaboatto

It worked better than expected:
https://internal1.languagetool.org/regression-tests/via-http/2024-09-03/pt-BR/
https://internal1.languagetool.org/regression-tests/via-http/2024-09-03/pt-PT/

With such a positive experience, it seems I will “play” more with the disambiguation from now on 😛 😛 😛 😛 😛 😛 😛

I AM SO HAPPY!!!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants