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

Misc tidying #2182

Merged
merged 3 commits into from
May 17, 2024
Merged

Conversation

ADBond
Copy link
Contributor

@ADBond ADBond commented May 14, 2024

Three small unrelated changes that weren't quite worth putting separately:

  • updated ColumnExpression typing to better clarify what allowed operations look like
  • remove a dialect dict we don't use anymore
  • move where we have functions for converting dicts to ComparisonCreator/ComparisonLevelCreator

On the last point I am not convinced if these are necessarily the best place to have these functions (maybe the base classes make more sense, but then more complicated as we need to make reference to the subclasses CustomXXX explicitly), but I think the current structure is a bit confusing (ComparisonCreator._convert_to_creator() makes a ComparisonLevelCreator), so now it is at least consistent across comparison + level.

@ADBond ADBond requested a review from RobinL May 14, 2024 15:40
@RobinL RobinL merged commit 9848f04 into moj-analytical-services:splink4_dev May 17, 2024
25 checks passed
Copy link
Member

@RobinL RobinL left a comment

Choose a reason for hiding this comment

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

Great!

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