-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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-PT] Improved antipattern in rule ID:POSSESSIVE_WITHOUT_ARTICLE #9376
Conversation
And another antipattern for other rule (id:PARA-POR_TER_PARTICIPIO-PASSADO) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR includes disparate changes to unrelated rules, IMO it should be split into multiple PRs.
We've finished our (Git documentation)[https://dev.languagetool.org/pt/git] for Portuguese-language contributors, do have a look, it might help guide you.
@@ -1398,6 +1398,12 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA. | |||
|
|||
<rulegroup id='GENERAL_GENDER_AGREEMENT_ERRORS' name="Concordância: Geral"> | |||
<url>https://pt.wikibooks.org/wiki/Portugu%C3%AAs/Concord%C3%A2ncia/Concord%C3%A2ncia_nominal</url> | |||
<antipattern> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This antipattern technically allows for:
o que você acha dos outros regras?
quero dar mais consistência aos outros regras
Both of which are incorrect, so I don't think we should be introducing this.
The key word here is the verb. If you have a verb that governs a certain preposition (such as 'impor', in your example), then we should allow these constructions. I'd suggest adding an extra token accounting for this verb.
@@ -8426,6 +8432,15 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA. | |||
<example>as bagagens não convencionais agora desaparecerão</example> | |||
</antipattern> | |||
|
|||
<antipattern> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The key blocker of agreement here is the verb in the first-person singular, not the whole construction. I'd rethink this.
<token postag='DP1.+' postag_regexp='yes'/> | ||
<token regexp='yes'>car[ao]s?|estimad[ao]s?|excelentes?|excelentíssim[ao]s?|querid[ao]s?</token> | ||
<token regexp='yes'>adoráv(el|eis)|amantes?|amig[ao]s?|car[ao]s?|clientes?|colaborador(es)?|colegas?|companheir[ao]s?|empregad[ao]s?|estimad[ao]s?|excelentes?|excelentíssim[ao]s?|filh[ao]s?|funcionári[ao]s?|inimig[ao]s?|irmã(s|o|os)?|mães?|médic[ao]s?|pais?|prim[ao]s?|professor(res)?|querid[ao]s?</token> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick 🥜] to avoid this type of thing, why not just a list of words with a POS tag filter for nouns/adjectives, and inflected="yes"
?
Sure, thanks for the feedback. I will fix everything at night, and tomorrow morning you will see the new code. |
Regarding the other rule “ir no ginásio” which I referred last week, it is too complex, and it won't be ready for this release of LanguageTool in a few days. |
Sure, there's no rush. Just try to keep unrelated changes on different branches (and therefore PRs), because we could already have merged a couple of your changes! 😢 |
Ahhhh… sorry… I thought I should only do that for rules and for APs add all in the same branch. We are always learning, I will do better. |
Aha, I understand your confusion. No, it's generally a better idea to keep everything separate. No worries, though, these APs should be easy enough to fix. |
Heya, I believe I have resolved the issues. Please have a look when you have the time. Regarding the I simply made the changes you requested and did a From now on, I will create a branch for each rule and for each AP, this will make reviewing easier. Thanks! |
My larger point here is that it is the 1SG inflection of the verb that makes that construction allowable. The fronting of the direct object is only incidental to that construction being incorrect. You can delete that rule, but we plan to do some larger work with agreement rules in the near future and I am pretty sure that we will greatly reduce the number of instances of 1SG verbs tagged.
I think you can keep antipatterns of the same rule within the same PR, no need to split them. Unless, of course, they are each massive and there's a high probability one of them will need more work. |
Heya, Susana and Pedro,
This enhancement fixes false positives that have been around for a very long time, and I should have done it before.
But it is better late, than never.
❤️ ❤️ ❤️ ❤️ ❤️