Open skirpichev opened 3 years ago
Thanks for the suggestion. Unfortunately, I have no idea how I could even begin to address this. The AST transformation example is just a proof of concept. Of course, if one uses ideas to modify the AST and also uses pytest, which also does AST transformations as you point out, there is no guarantee that the two types of transformations will, in general, be compatible. And, as you show, they (currently) are not.
Unfortunately, while it is an interesting suggestion, I will have to close this issue as being out of scope for this project, and beyond my capabilities. If you have concrete suggestions to offer as to how I might go about to address this, please feel free to reopen this issue.
On Tue, Jun 08, 2021 at 03:53:53AM -0700, André Roberge wrote:
If you have concrete suggestions to offer as to how I might go about to address this
Probably, pytest can adopt the ideas machinery to handle their ast transformation. That was my initial guess, probably there are other ways to handle this issue, maybe within the ideas project itself.
If you are ok with this solution - I'll look into to provide a patch (for pytest, but that might require some changes on the ideas side).
I would be ok with this solution. However, note that ideas is really just a toy project created by a dabbling amateur whereas pytest is an industry-grade project created by professional programmers: I would not be surprised at all if any patch submitted to pytest to solve this issue were to be rejected by the maintainers of that project. At the same time, I am open to anyone helping contributing to making ideas more robust and useful - and helping me learn more in doing so.
any patch submitted to pytest to solve this issue were to be rejected by the maintainers of that project
Probably, so. But I would like to see huge number of Rational(n, m)
in my tests being reduced to n/m
. Apparently, the pytest machinery is not flexible enough to add additional ast/token transformations - that's why I think they could use your framework.
If you do think this issue can't be solved in the ideas itself - feel free to close it. I'll open a PR if I find a way ;-)
Just for record, I corrected the example - it was wrong. The problem happens only for invalid asserts: tracebacks aren't very helpfull.
FYI: permission error can be fixed easily. In fact, I don't know why you open files for updating here: 'rb' mode seems to be more correct.
Unfortunately, pytest's tracebacks aren't very helpful with the ideas:
$ pytest test_fractions.py
============================= test session starts ==============================
platform linux -- Python 3.9.2, pytest-6.2.4, py-1.10.0, pluggy-0.13.0
rootdir: /home/sk/ideas
plugins: cov-2.10.1
collected 3 items
test_fractions.py ..F [100%]
=================================== FAILURES ===================================
_______________________________ test_arith_wrong _______________________________
> ???
E AssertionError
test_fractions.py:14: AssertionError
=========================== short test summary info ============================
FAILED test_fractions.py::test_arith_wrong - AssertionError
========================= 1 failed, 2 passed in 0.71s ==========================
$ cat test_fractions.py
import fractions
def test_not_float():
assert isinstance(1/2, fractions.Fraction)
def test_arith_correct():
assert 1/2 + 3/5 == fractions.Fraction(11, 10)
def test_arith_wrong():
assert 1/2 + 3/5 == fractions.Fraction(11, 12)
BTW, I think you may change the API (at least for AST transformations, but, perhaps - for everything) to be like the IPython ast_transformers option. I.e. a list of AST transformations to easily combine them. Currently your users forced to make custom transform_ast() function, which essentially just run sequentially all transformations. This is merely a boilerplate code. If this does make sense for you - I'll open a separate issue.
Reopening this issue to keep track of work to do.
Take an example (ideas were installed from the git):
Great!
But pytest does some magic AST transformation with the assert statement. Thus for following:
new test blow up: