BlockingRule: Clarify name of sql property

[RobinL]

Type of PR

• BUG
• FEAT
• MAINT
• DOC

Summary

This PR attempts to clarify the BlockingRule class. It is a step towards enabling ExplodingBlockingRules

It will be the first in a set of three PRs which incrementally add clarifications.

Clarify name of sql property

🤔 Previously BlockingRule had a sql property which was unused, and a property called blocking_rule which 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

Blocking rule init argument allowed blocking_rule to be BlockingRule, dict or str

🤔 This made the code hard to read/unpredictable code because types were unclear

Solution It's now a string only

Clarify name methos

🤔 Previously BlockingRule had a sql property and_not_preceding_rules_sql(self). Not very clear

Solution: I have clarified method names here toexclude_pairs_generated_by_this_rule_sql and exclude_pairs_generated_by_all_preceding_rules_sql. This is partly because these implementations will differnt in #1692

Finally I've also added type hinting to make refactoring a bit easier

[RobinL]

I don't think we should allow this to take as an input any of those types - i think this should be string only

I've just pushed a commit to enforce this temporariliy to check that tests still pass

[ThomasHepworth]

A wider thought for another PR/Splink 4 - I think apply_salt should be handled from within the BlockingRule class, not outside.

It wouldn't be too hard to set the dialect for each BR in a central settings class/area when we go to unpack the settings dictionary (assuming we are planning to continue forward with that).

It's generally bad practice to have high 'coupling' (dependence between functions), which this is a perfect example of - changes to the salting methodology needs to be closely mirrored in the function linked above.

On top of that, it's quite hard to follow if you're new to the code base.

It would be better if we simply had to call br.salted_blocking_rules to get the output. Whether it came from a SparkLinker would then ideally be handled by the BlockingRule class (or a salting subclass).

[ThomasHepworth]

This looks good!

[ThomasHepworth]

Might as well add the int type hint here too.

[RobinL]

Good opint. I'll add it to the next PR, which changes this a bit anyway

[ThomasHepworth]

My comment above is already being addressed by - #1701. Glad to see we're thinking the same thing.