Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix - key error for a special case such as "뚫리다" #16

Merged
merged 3 commits into from
Jan 17, 2023

Conversation

Min-Ho-Lim
Copy link
Contributor

Summary

Apparently, while I was working on my personal project, I need a simple romanization library for Korean language. And, I found this library. However, I encountered one edge case and this library does not support.

For instance, if a word has a double constant which is "ㅀ" and the next syllable starts with NULL_CONSONANT. It will throw a Key-error as #15 described. It is because there is no logic to handle such case in `pronouncer.py'

So, I created a logic to handle such cases.

Changes

  1. Added a logic to handle a special case a syllable has "ㅀ" and the next syllable starts with NULL_CONSONANT
  2. Added test_double_consonant_final_and_next_syllable_not_null_initial in test_romanizer.py to prevent this situation in the future.
  3. Applied prettier for some files.

Screenshots

Pytest Result

image

@vercel
Copy link

vercel bot commented Dec 26, 2022

Someone is attempting to deploy a commit to a Personal Account owned by @osori on Vercel.

@osori first needs to authorize it.

@vercel
Copy link

vercel bot commented Jan 17, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
korean-romanizer ❌ Failed (Inspect) Jan 17, 2023 at 3:44PM (UTC)

@osori
Copy link
Owner

osori commented Jan 17, 2023

Hey! Thank you so much for taking time to fix this bug and providing all the details. I also appreciate the tests you wrote and the linting you applied. Merging this, thanks for your contribution 🥇

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants