Skip to content

Hoist by_name() matching helpers out of template into importable code - #106

Merged
gefleury merged 2 commits into
openMetadataInitiative:pipelinefrom
apdavison:by-name-helpers
Oct 9, 2026
Merged

gefleury merged 2 commits into
openMetadataInitiative:pipelinefrom
apdavison:by-name-helpers

Conversation

@apdavison

Copy link
Copy Markdown
Member

This makes the code easier to test, and makes it reusable, specifically by fairgraph.

This makes the code easier to test, and makes it reusable, specifically by fairgraph.
@apdavison
apdavison requested a review from gefleury September 12, 2026 21:56
@apdavison apdavison added the enhancement New feature or request label Sep 12, 2026
Comment thread pipeline/tests/test_name_matching.py Outdated
matches_name(name, query, match, case_sensitive, ignore_accents, ignore_separators)
for name in names
if name is not None
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

test_agreement asserts nothing when found is None: should it also check that no instance's name-like properties/synonyms satisfy matches_name() for the query in that case?

Check both directions of the `by_name()`/`matches_name()` agreement. `test_agreement` only checked that every instance `by_name()` returned did match the query, so whenever `by_name()` found nothing the test asserted nothing at all: 8 of the 24 parameter combinations, namely every one with `ignore_accents=False` and `match` of "equals" or "contains".

It now builds the expected list independently from `instances()` and asserts equality of sorted lists, so an instance that should have been found but wasn't is caught too, and an empty result is required to be `None` rather than an empty list.
@gefleury
gefleury merged commit 186fe7a into openMetadataInitiative:pipeline Oct 9, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants