Skip to content

Add ignore_accents option to by_name() - #103

Open
gefleury wants to merge 4 commits into
openMetadataInitiative:pipelinefrom
gefleury:by-name-accent-handling
Open

Add ignore_accents option to by_name()#103
gefleury wants to merge 4 commits into
openMetadataInitiative:pipelinefrom
gefleury:by-name-accent-handling

Conversation

@gefleury

Copy link
Copy Markdown

Adds accent-insensitive matching to by_name(), a follow-up to the case_sensitive and match options from #100.

  • New ignore_accents parameter (default False, so existing behaviour is unchanged).
  • When True, accents and other diacritical marks are stripped before matching (e.g. "Republique francaise" matches "République française"). Combine with case_sensitive=False to absorb case differences as well.
  • Uses NFD (accents only), not NFKD, so Unicode compatibility folding (e.g. superscripts
    like m3) is deliberately not applied here. That's a separate concern that could
    be handled later if needed.
  • The generated modules now import unicodedata (added to the preamble in translator.py
    only for modules that have by_name).
  • One test added in test_regressions.py

by_name() gains an ignore_accents parameter (default False). When True, accents and other diacritical marks are stripped before matching,
The generated modules import unicodedata for this.
cls._instance_lookup[key] = [instance]

def remove_accents(s):
nfd_form = unicodedata.normalize("NFD", s)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This doesn't handle Ł ø đ æ ß ə ı

For example:

>>> import unicodedata
>>> s = "Łódź"
>>> unicodedata.normalize("NFD", s)
'Łódź'

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

You're right, I missed it.

  • "NFKD" (instead of "NFD") doesn't help either
  • unidecode handles those letters
from unidecode import unidecode

for s in ["Ł", "ø", "đ", "æ", "ß", "ə", "ı", "µA", "m³", "Ελλάδα"]:
    print(f"{s} -> {unidecode(s)}")

### Output
Ł -> L
ø -> o
đ -> d
æ -> ae
ß -> ss
ə -> @
ı -> i
µA -> uA
m³ -> m3
Ελλάδα -> Ellada

but it adds a runtime dependency and transliterates too aggressively (e.g. it maps ə to @, which is wrong for eg Azerbaijan's synonym "Azərbaycan").

  • My preferred option: keep NFD for accents and add a small explicit map (no dependency, full control):
_SPECIAL = str.maketrans({
    "Ł": "L", "ł": "l",
    "Ø": "O", "ø": "o",
    "Đ": "D", "đ": "d",
    "Æ": "AE", "æ": "ae",
    "ß": "ss",
    "Ə": "E", "ə": "e",
    "ı": "i",
})

def remove_accents(s):
    s = s.translate(_SPECIAL)                 
    nfd_form = unicodedata.normalize("NFD", s)
    return "".join(c for c in nfd_fo

Happy to implement it this way if you agree

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes, I'm happy with that approach

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

OK. It's now implemented in 21e0397 with an updated test.

Comment thread pipeline/translator.py Outdated
}
extra_imports = set()
if has_instances:
extra_imports.add("import unicodedata")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since unicodedata is used in only one specific place in the code, it might be better to import unicodedata inside the remove_accents() function.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, that's simpler. Done in 3eed40f

remove_accents() only stripped accents that Unicode can split off from their base letter (NFD normalization), so letters with no such decomposition (ß, œ, æ, ø, ł, đ, etc.) passed through unchanged.
Add an explicit translation map, applied after the NFD strip so composed forms (e.g. "ǿ") are also handled correctly. Updated test.
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.

2 participants