cgravill
commented
3 years ago In short, yes :). I quite like the lambda! (And it certainly beats the double class hierarchy in RLO).
Super, I'll keep going to make this possibly correct!
I think it's nice for the Match to contain the actual Rule(Matcher) object as a field, just for reference. But we can put the callable alongside that. Also, I think the "rule_specific_data" in the Match is no longer needed - it is embedded within the Callable ?
Yes, it's embedded. Plan would be to do away with that entirely if this works.
The original design gave each RuleMatcher subclass an "apply_at" method partly because it seemed like a nice division, too - between (a) finding matches and (b) doing the rewrites that were found - but removing the abstractmethod in the superclass might be enough to make mypy happy, with each subclass:
- declaring it's own differently-typed
apply_at- creating Callable's that call only that class's
apply_at, with appropriate argumentsand that might constitute a smaller change, that still captures the essential bit with the Callable?
Also probably good to separate out from the other typechecking changes to follow.
Agreed, I hope to merge #920 to master soon, then to here. I could have done this change separately which might have been simpler to review.
Follow on from #920
Instead of creating a Match with a variable number of custom parameters then dispatched by an ABC, then filling in kwargs. Can we instead just create a callable and let it be run on demand?
The benefit is that it keeps the parameter handling local, so could do away with lots of the comments. It's going to type check, which the "specialisation" of kwargs isn't accepted by mypy, even if we add kwargs to the overriding functions.