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

Fix s plural noun #168

Merged
merged 6 commits into from
Jan 19, 2023
Merged

Fix s plural noun #168

merged 6 commits into from
Jan 19, 2023

Conversation

george-gca
Copy link
Contributor

Fixes #161

Copy link
Owner

@jaraco jaraco left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for the contrib. Can you add a test that captures the known bad cases (fails before and passes after this change)?

@george-gca
Copy link
Contributor Author

Sure, but some of the failing cases are already there. I gave a quick look at your testing code and couldn't understand it, so I need to do a deep dive. Does your test code test the opposite, and also the same case? I mean, for example:

bass -> basses

Does your testing code tests

singular(bass)
plural(bass)
singular(basses)
plural(basses)

?

@jaraco
Copy link
Owner

jaraco commented Oct 22, 2022

Sure, but some of the failing cases are already there. I gave a quick look at your testing code and couldn't understand it, so I need to do a deep dive. Does your test code test the opposite, and also the same case? I mean, for example:

bass -> basses

Does your testing code tests

singular(bass)
plural(bass)
singular(basses)
plural(basses)

?

I'm not sure, but I'm guessing not. Historically, the project assumes valid inputs, so a call of singular(bass) or plural(basses) is considered invalid. That is, the project doesn't perform general-purpose semantic language comprehension, but instead provides simple inflection given known inputs.

I'm happy to accept more sophistication, but the status quo is somewhat limited.

@george-gca
Copy link
Contributor Author

Actually some of the failing cases are like you said, singular(bass) or plural(basses). I will give it a thought about how to do some kind of verification before doing singularization/pluralization.

@george-gca
Copy link
Contributor Author

Added test cases like jeans -> jeans

@jaraco jaraco merged commit 7ad43ac into jaraco:main Jan 19, 2023
jaraco added a commit that referenced this pull request Jan 19, 2023
@jaraco
Copy link
Owner

jaraco commented Jan 19, 2023

Oops. I merged this not double-checking the tests and I see now they're failing, so I've force-pushed without those changes. Please re-submit.

@george-gca george-gca deleted the fix_s_plural_noun branch July 6, 2023 15:40
@george-gca george-gca restored the fix_s_plural_noun branch July 6, 2023 16:06
@george-gca george-gca deleted the fix_s_plural_noun branch July 6, 2023 16:06
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.

Words ending in 's' should never be pluralized by adding another 's'
2 participants