-
Notifications
You must be signed in to change notification settings - Fork 907
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
Fixes the incorrect stemming of the verb "revocares" and of a word that looks like a verb but is not #21999
base: trunk
Are you sure you want to change the base?
Conversation
…s morphologydata file to avoid overstemming words that look verbs but are not
… if statements checking for accented words
Pull Request Test Coverage Report for Build 9e0b93fe464dab0056be8406f85a53fb90ddd0caDetails
💛 - Coveralls |
…enerateStem file for Spanish
…panish-verb-stemmed-incorrectly
@@ -10,6 +10,7 @@ const wordsToStem = [ | |||
[ "martes", "martes" ], | |||
[ "microondas", "microondas" ], | |||
[ "jesús", "jesus" ], | |||
[ "práxedes", "praxedes" ], |
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.
Out of curiosity, why use [ "práxedes", "praxedes" ], as a example? 🤔 As far as I know, Práxedes is a name of Greek origin but it isn't really used in Spanish. I would consider using a more common name in Spain like ["Ines",
"Inés"], ["Elias", "Elías"], or other nouns that end with an -s but aren't plurals like: crisis, tesis, análisis, dosis, atlas, caos, campus...
@@ -73,7 +74,7 @@ const wordsToStem = [ | |||
[ "comienzo", "comenz" ], | |||
// Input a word that ends in a common verb suffix. | |||
[ "saltaron", "salt" ], | |||
// [ "revocares", "revoc" ], | |||
[ "revocares", "revoc" ], | |||
// Input a word that ends in -os, -s, -a, -o, -á, -í,-ó, -é, -e. | |||
[ "agostinas", "agostin" ], | |||
[ "boboré", "bobor" ], |
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.
Maybe consider adding some more examples like i.e. [ "albatros", "albatros" ], [ "objetivo", "objet" ], [ "bisturí", "bistur" ], [ "dominó", "domin" ] ?
// [ "cabalgada", "cabalgad" ], | ||
[ "abacería", "abaceri" ], | ||
[ "lugar", "lugar" ], |
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.
I don't think [ "lugar", "lugar" ] is a good example since there is no verb as "lugar". Maybe consider other examples like i.e. [ "cántaro", "cántar" ], [ "llanta", "llant" ], [ "vela", "vel" ] ?
@iolse nice work! I've done the ACT 👍 |
Context
Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
calculateCoverage
file it now states:The current coverage of the Spanish stemmer is 99.97541185148758 %. The number of errors is 2.
, the two errors in question refer in fact to the correct stemming of the words lugar and práxedes , which where incorrectly stemmed in the previous version of the stemmer.yoastseo/package.json
but inyoastseo/jest.config.js
. Also, instructions for formatting have been updated to avoid indentation in the goldStandard list.Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
yarn test
and make sure that everything passesuse morphology
tag onTest words that are not verbs
Relevant test scenarios
Test instructions for QA when the code is in the RC
QA can test this PR by following these steps:
Impact check
This PR affects the following parts of the plugin, which may require extra testing:
UI changes
Other environments
[shopify-seo]
, added test instructions for Shopify and attached theShopify
label to this PR.Documentation
Quality assurance
Innovation
innovation
label.Fixes https://github.com/Yoast/lingo-other-tasks/issues/234