Add Implication and Remove Nesting#27
Conversation
|
This looks pretty good to me! Can you throw in some tests here to make sure things work (mainly useful for future regressions). Be sure to test multiple children in the left/right formula (e.g., |
There was a problem hiding this comment.
Nice additions!
Tox/flake8 has some complaints about formatting, can you try to fix those? CONTRIBUTING.md has instructions for running it but let me know if it's unclear.
After you fix the formatting it might complain about Mypy, I left tips for that in the review.
blyxxyz
left a comment
blyxxyz
left a comment
There was a problem hiding this comment.
Great! The test passes, and mypy passes, so this is almost done.
flake8 still has a few complaints:
nnf/__init__.py:17:1: F401 '_typeshed.IdentityFunction' imported but unused
nnf/__init__.py:87:1: E305 expected 2 blank lines after class or function definition, found 1
nnf/__init__.py:118:39: E225 missing whitespace around operator
nnf/__init__.py:1741:1: E305 expected 2 blank lines after class or function definition, found 1
Tox should also list those problems, have you managed to get it set up?
(We should probably add CI so Github runs the checks automatically when you open a PR, that makes things easier.)
Co-authored-by: Jan Verbeek <jan.verbeek@posteo.nl>
blyxxyz
left a comment
blyxxyz
left a comment
There was a problem hiding this comment.
Looks good. Thank you for bearing with me.
|
No worries, thanks for taking a look! Hope it proves to be useful in the long run :) |
3 participants
-Adds >> functionality to Var
-Creates an option to auto-simplify formulas upon creation by changing
auto_simplifyin the init module to True