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

Fix ColumnsReversedLevel #2395

Merged
merged 2 commits into from
Sep 11, 2024
Merged

Conversation

RobinL
Copy link
Member

@RobinL RobinL commented Sep 11, 2024

Fixing issue described in following discussino:

Question re: cll.ColumnsReversedLevel

At the moment cll.ColumnsReversedLevel("first_name", "surname").get_comparison_level("duckdb").as_dict() yields:

 'label_for_charts': 'Match on reversed cols: first_name and surname'}```

But one real-world application would be to set up:

* a first name comparison something like :
    * exact match first name
    * exact match first name to surname
    * fuzzy match first name
    * else
* a surname comparison somehting like:
    * exact match surname
    * exact match surname to first name
    * fuzzy match surname
    * else

with that in mind, in that example I think you’d want a ‘one way’ columns reversed level

i.e.

```{'sql_condition': '"first_name_l" = "surname_r"',
 'label_for_charts': 'Match on reversed cols: first_name and surname'}```

Have I got that right?  I think otherwise it’s effectively an ‘exact match full name with a reversal’, which can be used in a comparison that tries to encompass the full name (`cl.ForenameSurnameComparison`) but not if names are split out into multiple comparisons

My proposed solution would be to add a ‘symmetric’ argument to the `ColumnsReversedLevel` which by dfault is False.  If activated, it compares in both directions

@RobinL RobinL merged commit 115b76f into master Sep 11, 2024
25 checks passed
@RobinL RobinL deleted the add_symmetrical_arg_to_columns_reversed branch September 11, 2024 08:11
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.

1 participant