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

remove_accents: refactor to NFKD normalization #464

Merged
merged 1 commit into from
Feb 6, 2023

Conversation

bosd
Copy link
Collaborator

@bosd bosd commented Feb 5, 2023

Refactor the remove accents functions to NFKD normalization form. So référence get normalized to reference.

This fixes: #444

Refactor the remove accents functions to NFKD normalization form.
So référence get normalized to reference.
@rmilecki
Copy link
Collaborator

rmilecki commented Feb 6, 2023

Looks good, could we also add some test for that?

@bosd
Copy link
Collaborator Author

bosd commented Feb 6, 2023

Looks good, could we also add some test for that?

I agree that it is advisable to cover it in tests.

My preference is to fasttrack this one. Generate a new release on pypi. As there are some minor bugs in the current release.

Then in a separe PR create more advanced tests for the normalize function.
(I am propably not the one to write these advanced tests because we don't have it in my language.)
I could imagine someone wants to add test for german umlaut (was mentioned somewhere here)
As well as the polish & french language might hase some characters which might be affected by this..

@rmilecki If you are ok, with fasttracking this one in the current form w/o tests. Please merge this one 👍

@rmilecki rmilecki merged commit 0938680 into invoice-x:master Feb 6, 2023
@bosd bosd deleted the unidecode-NFKD branch February 6, 2023 12:45
@bosd
Copy link
Collaborator Author

bosd commented Feb 18, 2023

Sadly, This is not a correct solution. It now also deletes the € sign.

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.

remove_accents option does not work properly anymore
2 participants