Experiment with property testing

chalk-diagrams / chalk

A declarative drawing API in Python

MIT License

282 stars 13 forks source link

Experiment with property testing #111

Closed srush closed 2 years ago

srush commented 2 years ago

This is a fix for https://github.com/chalk-diagrams/chalk/issues/109

I tried to follow the haskell doc string more closely. The issue is how traces and envelopes should behave for non-unit vectors.

Unit tests assert that envelopes should bound traces from the origin. I believe this property is correct since each trace represents exactly one point on the left hand side of this equation.

‖proj(u, v)‖ / ‖v‖ ≤ dia.envelope(v) for all dia, u ∈ dia and v ∈ R²

srush commented 2 years ago

One issue I ran into in this PR is that python complains about recursion limit more often now. Might need to make sure that some of the core transforms use less recursion.

danoneata commented 2 years ago

One issue I ran into in this PR is that python complains about recursion limit more often now.

I guess this happens because now the envelope is wrapped twice (self is called twice now, once in apply_linear and once in apply_translation; while before it was called only a single time)? Other than that, I don't see what could cause the RecursionError compared to the previous code.

srush commented 2 years ago

Yup, good catch. I will fix that. It seems like recursion is fine in general, but we should be careful in the core library that gets called everywhere.

danoneata commented 2 years ago

Yup, good catch. I will fix that.

Yeah, that's unfortunate; I liked the logic separation into apply_translation and apply_linear that you had previously introduced.

It seems like recursion is fine in general, but we should be careful in the core library that gets called everywhere.

Agreed!

A couple minor points:

Could we also pass the tests directory to black, isort, flake, etc. in the Makefile?
There are two remnant print statements in frame from padding.py, which probably where used for debugging and could be removed.

Other than that very nice job on the tests! They were much needed.

srush commented 2 years ago

Good point. I was able to type check the tests as well. Hypothesis is a really nice library.

Now that we have a way to sample diagrams, might be interesting to think up some of the other properties to check in the core lib. For example, how should besides change the envelopes and trails. (it's possible someone has done this already as well).