-
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] Removed "temp_off" from rule ID:OS_DOIS_AS_DUAS_AMBOS_AMBAS #10870
Conversation
WalkthroughThis pull request modifies an XML configuration file that defines language rules for Portuguese. The primary change involves the removal of the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Some matches are still not sitting quite right to me:
This sounds odd because the output draws attention to the number of letters written, whereas the original question seems to put more emphasis on who wrote them. Maybe let's prevent this rule from matching questions starting with interrogative pronouns (Quem/Quando/Onde/Como). Also odd:
The output diminishes the singular focus present in the adjective The same problem occurs in more complex comparative/superlative sentences, such as:
I would rather not have us suggesting "ambas" next to "Só" because it removes the numeric importance of "as duas" here. Think about it like this: if the original was "Só as três fábricas", we couldn't say "Só todas as fabricas", for instance, because it cancels the singularity proposed by the word "só". The same sort of numeric focus is removed from this example, though it would perhaps be more challenging to resolve:
|
Ahhhh… that will require more effort/time, I will do it at 5am. Tomorrow it will be fixed, I hope. Thank you for the feedback! When the results are too many I lose myself checking one by one. Thanks! |
I have fixed the FPs you reported above and readded "temp_off". |
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.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
languagetool-language-modules/pt/src/main/resources/org/languagetool/rules/pt/style.xml (1)
Line range hint
4-6
: Good start, but consider additional antipatterns.The addition of this antipattern helps address some of the concerns raised in the PR comments by preventing matches for specific article-adjective combinations that could lead to incorrect suggestions. This is a step in the right direction towards improving the rule's accuracy.
However, the PR comments also highlight other problematic cases, such as comparative or superlative contexts, which may not be covered by this antipattern. Consider adding more antipatterns to address these remaining concerns and further refine the rule's behavior.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- languagetool-language-modules/pt/src/main/resources/org/languagetool/rules/pt/style.xml (3 hunks)
Additional comments not posted (2)
languagetool-language-modules/pt/src/main/resources/org/languagetool/rules/pt/style.xml (2)
Line range hint
1-3
: LGTM!The addition of this antipattern effectively addresses the concern raised in the PR comments about the rule incorrectly matching questions starting with interrogative pronouns. By preventing matches for interrogative words followed by a question mark, the rule's accuracy is improved, reducing false positives.
Line range hint
7-8
: Provide rationale for new exceptions and verify impact.The addition of new exceptions for the token "precisar" and other adjectives aims to enhance the specificity of the rules regarding the usage of "as duas" and "os dois." However, without more context about the motivation behind these specific exceptions, it's difficult to assess their impact on the rule's accuracy.
Could you please provide more information about the rationale for including these particular exceptions? Additionally, I suggest thoroughly verifying the impact of these changes on the rule's behavior to ensure they improve the overall accuracy and don't introduce unintended consequences.
To verify the impact of the new exceptions, consider running the following script:
Heya, @susanaboatto and @p-goulart ,
I have removed the temp_off:
https://internal1.languagetool.org/regression-tests/via-http/2024-09-09/pt-BR_full/result_style_OS_DOIS_AS_DUAS_AMBOS_AMBAS%5B1%5D.html
https://internal1.languagetool.org/regression-tests/via-http/2024-09-09/pt-BR_full/result_style_OS_DOIS_AS_DUAS_AMBOS_AMBAS%5B2%5D.html
The results are too many for me to focus on checking one by one, but they look okay.
Thanks!
😛 😛 😛 😛 😛 😛 😛 😛 😛 😛 😛 😛 😛 😛 😛 😛 😛
Summary by CodeRabbit
New Features
Bug Fixes