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

refactor pluralize/singularize inflection logics #62

Merged
merged 18 commits into from
Jan 6, 2023

Conversation

sio4
Copy link
Member

@sio4 sio4 commented Jan 2, 2023

What is being done in this PR?

  • refactor the data structure and the logic for the singular/plural inflection
  • improved test cases and test logic for the same target

What are the main choices made to get to this solution?

  • the simple logic for the singular/plural inflection is not good enough for the current data size
  • we need code comments for the organized data. without it, it cannot be maintainable
  • we need a new data structure to handle exceptional data efficiently

What was discovered while working on it? (Optional)

  • English is toooooooo hard

List the manual test cases you've covered before sending this PR:

  • all previous test cases were preserved basically
  • but for some words, I found the data is not correct or needs to be corrected with the trend

fixes #61

Note that:
This is not huge but a kind of big change since the logic, data structure, and actual data are changed at the same time in a single PR. However, I tried to keep it simple per each commit to make it easy to track the whole change direction.

@sio4 sio4 added enhancement New feature or request internal internal cleanup (or a kind of refactoring) labels Jan 2, 2023
@sio4 sio4 added this to the v1.0.0 milestone Jan 2, 2023
@sio4 sio4 self-assigned this Jan 2, 2023
@sio4 sio4 marked this pull request as draft January 2, 2023 09:38
@sio4 sio4 marked this pull request as ready for review January 5, 2023 17:19
@sio4 sio4 requested a review from a team January 5, 2023 17:19
@sio4
Copy link
Member Author

sio4 commented Jan 5, 2023

Hi @paganotoni , Happy New Year!

I had a "hard" time with this PR to fix the internal structure of this package. There were basically two issues:

  • English inflection for the plural is hard (at least for me, but I cannot fix it)
  • Flect has been grown for years by many contributors, patching one by one, and it became hard to maintain.

My direction for this PR was to make Flect has some rules for adding inflection rules, hard-coded words and exceptions, and test cases with an organized structure.

Please take a look at this PR. FYI, I tried to make it simple to review by isolating each change and each rule update within a single commit.

I would like to release it as v1.0.0 when you review and I fix all review comments if any.
Thanks!

@sio4
Copy link
Member Author

sio4 commented Jan 6, 2023

References:

#!/bin/bash

end=$1

[ "$end" = "" ] && {
	echo "usage: $0 end"
	exit
}

curl https://www.wordmom.com/nouns/that-end-with-$end 2>/dev/null \
  |grep "[a-z]*$end</li" \
  |sed 's,<li [^>]*>,https://en.wiktionary.org/wiki/,g;s,</li>,#English\n,g' \
  |grep https

@paganotoni
Copy link
Member

Hey @sio4! Thanks for taking care of this one. Happy new year. It looks good to me.

@paganotoni paganotoni merged commit 17261fe into main Jan 6, 2023
@paganotoni paganotoni deleted the refactoring-with-bugfix branch January 6, 2023 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request internal internal cleanup (or a kind of refactoring)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Singular form of "invoices" is "invoice"
2 participants