You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In my case, I'm adding typing to cmsh. I have a Variable class which is essentially a bool except it is backed by a variable in the CNF passed to the SAT solver. Its value isn't available until the model is solved. I'm wanting to use Variable and bool interchangeably, so building a CNF model looks like building a complex boolean expression (except you have to use bitwise operators, e.g., & instead of and). This means that if you specify some previously unspecified portion of your inputs, it gets evaluated in Python (and you get native types back), rather than having to wait until the Solve() call. Ultimately, this gives you a smaller model.
Anyhow, so the basic and implementation that supports both Container and bool works with isinstance:
The problem with this is that there are double the isinstance calls than you need: you only need two if you assign them to variables. According to cProfile on some of my code which uses cmsh, isinstance is the most called method because I'm constructing fairly large CNF models.
When you assign the isinstance calls to variables, mypy quits inferring the types of left and right, resulting in errors:
$ mypy --strict ./main.py
main.py:51: error: No overload variant of "__and__" of "bool" matches argument type "Container"
main.py:51: note: Possible overload variants:
main.py:51: note: def __and__(self, bool) -> bool
main.py:51: note: def __and__(self, int) -> int
main.py:51: error: Unsupported operand types for & ("Container" and "bool")
main.py:51: note: Both left and right operands are unions
(Line 51 is the return left & right line from the v_and call. It thinks one and/or both of the arguments can be bool; however, they can only be Container.)
We can potentially support this assuming you annotate lbool and rbool as Final. But this is tricky and therefore low priority.
Also, in almost all Python programs (except heavy number crunching), isinstance() is one of the most called functions. This is because of how Python works (for example it is called after creation of every instance to determine whether __init__() should be called on what is returned by __new__()) so I wouldn't be too worried about this.
Oh, definitely agree, isinstance() will be high up there. A lot of these instances though are from my code, since my main dependency (pycryptosat) is a C API. And a number of them are redundant (like this case). It'd be nice to have both typing and speed, but I'll get there with more work. Just thought this was a case for later improvement.
And yeah, no rush. :) Thanks for taking a look at this! I appreciate all the work that's gone into this project.
I'm filing this as a bug, but feel free to call it a feature request. :)
Lets say you have a basic container class:
In my case, I'm adding typing to
cmsh
. I have aVariable
class which is essentially abool
except it is backed by a variable in the CNF passed to the SAT solver. Its value isn't available until the model is solved. I'm wanting to useVariable
andbool
interchangeably, so building a CNF model looks like building a complex boolean expression (except you have to use bitwise operators, e.g.,&
instead ofand
). This means that if you specify some previously unspecified portion of your inputs, it gets evaluated in Python (and you get native types back), rather than having to wait until theSolve()
call. Ultimately, this gives you a smaller model.Anyhow, so the basic
and
implementation that supports bothContainer
andbool
works withisinstance
:This passes mypy fine in strict mode.
The problem with this is that there are double the
isinstance
calls than you need: you only need two if you assign them to variables. According tocProfile
on some of my code which usescmsh
,isinstance
is the most called method because I'm constructing fairly large CNF models.When you assign the
isinstance
calls to variables,mypy
quits inferring the types ofleft
andright
, resulting in errors:Which when type checked gives:
(Line 51 is the
return left & right
line from thev_and
call. It thinks one and/or both of the arguments can bebool
; however, they can only beContainer
.)I'm running:
Is there a different or better way of doing this? Thanks!
The text was updated successfully, but these errors were encountered: