Closed NicolasRouquette closed 1 year ago
I realize that I forgot to add support for parentheses. Please don't merge yet.
@iincer Do you have any ideas how to handle this type checking problem?
pdm run duty check-types
✗ Type-checking (1)
> mypy --strict --allow-any-generics --implicit-reexport --allow-untyped-calls --config-file=config/mypy.ini src tests
src/pacti/terms/polyhedra/grammar.py:597: error: Unsupported left operand type for << ("ParserElement") [operator]
This problem involves pyparsing.Forward
construct and the associated <<=
operator here:
paren_terms <<= pp.Group("(" + terms + ")").set_parse_action(_parse_paren_terms).set_name("paren_terms_contents")
Strangely, the type error refers to <<
even though the source code uses <<=
.
Changing to the following makes no difference in the type checker error message nor in the behavior.
According to the doc, <<=
is recommended over <<
.
paren_terms <<= pp.Group("(" + terms + ")").set_parse_action(_parse_paren_terms).set_name("paren_terms_contents")
There is some done about this topic:
https://pyparsing-docs.readthedocs.io/en/latest/HowToUsePyparsing.html#expression-operators https://pyparsing-docs.readthedocs.io/en/latest/HowToUsePyparsing.html#special-subclasses
There are several examples of usage of this construct; see for example these search results:
https://github.com/search?q=repo%3Apyparsing%2Fpyparsing+%3C%3C%3D&type=code&p=2
Alas, it is unclear whether this project is performing type checking though there are several type annotations in the source code.
Is this <<=
operator corresponding to the __lshift__
method here: https://github.com/pyparsing/pyparsing/blob/master/pyparsing/core.py#L5395
Hi @NicolasRouquette, I made an update to this branch. The problem was that the function pp.Forward().set_name()
in
paren_terms = pp.Forward().set_name("paren_terms")
returns a type which is a parent of pp.Forward
, and this parent class does not support the operator in question--the operator is defined for pp.Forward
. The solution was to cast the type to a subtype.
@iincer This PR is now ready for review.
Review notes:
*
as a multiplication operation for an absolute term coefficient.leq
and geq
expressions, add a check in the serializer to verify that all absolute term coefficients are positive after refactoring into a <= <constant>
form.eql
expression type checks (the sides are terms
)Review comments:
polyhedra.syntax
namespacegrammar.py
to polyhedra.syntax
polyhedra.syntax.data.py
Fixes #129