potassco / clingo

🤔 A grounder and solver for logic programs.
https://potassco.org/clingo
MIT License
599 stars 79 forks source link

mypy AST Callbacks #434

Closed MaxOstrowski closed 1 year ago

MaxOstrowski commented 1 year ago
    class RuleVisitor(Transformer):
        def visit_Rule(self, stm: AST) -> AST:
            return stm

    ruler = RuleVisitor()
    parse_string("a:- b.", ruler)

mypy --strict complains about:

error: Argument 2 to "parse_string" has incompatible type "RuleVisitor"; expected "Callable[[AST], None]"  [arg-type]
note: "RuleVisitor.__call__" has type "Callable[[Arg(AST, 'ast'), VarArg(Any), KwArg(Any)], AST]"

Is there anything we could do about it ? Is it just the return type that is off ? I'm not sure that I completely get it.

rkaminsk commented 1 year ago
    class RuleVisitor(Transformer):
        def visit_Rule(self, stm: AST) -> AST:
            return stm

    ruler = RuleVisitor()
    parse_string("a:- b.", ruler)

mypy --strict complains about:

error: Argument 2 to "parse_string" has incompatible type "RuleVisitor"; expected "Callable[[AST], None]"  [arg-type]
note: "RuleVisitor.__call__" has type "Callable[[Arg(AST, 'ast'), VarArg(Any), KwArg(Any)], AST]"

Is there anything we could do about it ? Is it just the return type that is off ? I'm not sure that I completely get it.

I think the error message is correct and points at a problem with your code. The function should not return a value. You should rather use something like this:

class RuleVisitor(Transformer):
    def visit_Rule(self, stm: AST) -> AST:
        return stm

ruler = RuleVisitor()
def do(stm):
    stm = ruler(stm)
    # do something with the transformed statement
    # (maybe add it via a builder)
    print(stm)
parse_string("a:- b.", do)
MaxOstrowski commented 1 year ago

Thank you for the explanation. I used the Transformer as a visitor

Note that the class works like a visitor if only self references are returned from such functions. But I agree that this is something parse_string can't know. Currently my visitor does assertion checks, so I think I'm ok with silencing the error. Thanks

rkaminsk commented 1 year ago

If you want to ignore the return value, you should wrap it in a function that explicitly does so. Maybe even with a comment. Silencing type errors is not a good idea, if you intend to use mypy.