(WIP) Refactor blocking rule for clarity #1699
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Type of PR
Summary
This PR attempts to clarify the BlockingRule class. It is a step towards enabling ExplodingBlockingRules
Things changed:
Clarify name of sql property
🤔 Previously
BlockingRule had a
sqlproperty which was unused, and a property called
blocking_rulewhich returned the sql. The later was confusing: did it return a
BlockingRule` or a sql string?.Solution: There's now a single property called
blocking_rule_sql
Better iteration
-🤔 When
UNION ALL
ing blocking rules, iteration was difficult because a salted blocking rule produces a list of blocking rules asbr.salted_blocking_rules
, but if it was not salted, you needed to access the blocking rules directly.So iteration looks like this:
Solution: To access all blocking rules and salted blocking rules, the user should now use
br.salted_blocking_rule_segments
, which is iterable (has 1 element in the case of no salting)Moving SQL up into BlockingRule class
Previously the SQL statement to be
UNION ALL
ed didn't live with the BlockingRuleThat was fine when there was only one variant, but to allow
[ExplodingBlockingRules](https://github.com/moj-analytical-services/splink/pull/1692)
this SQL will change depending on the type of blocking rule