fix custom magic methods and add tests #785
Conversation
| return False | ||
| for v1, v2 in zip(self, other): | ||
| for a, b in zip(v1, v2): | ||
| if math.fabs(a - b) > tol: |
There was a problem hiding this comment.
should we not replace this with the precision setting of compas?
noticed that we are not doing this for the other checks either...
There was a problem hiding this comment.
Yeah, that makes sense.
There was a problem hiding this comment.
actually, i'm a little confused by this comment. isn't compas.PRECISION more about formatting the way objects are printed rather than prescribing tolerances on computations?
There was a problem hiding this comment.
yes and no. it is, for example, also used in the geometric maps to determine comparison precision...
but i agree that there is no "official policy" for this at the moment.
There was a problem hiding this comment.
but in the tests, when geometry comparisons are concerned, we tend to use (or should be using) allclose to avoid tests failing for lack of precision
There was a problem hiding this comment.
yeah, i've changed to using allclose now, but that also uses a hardcoded tolerance...
maybe once there is an "official policy" on this, such changes can be made more thoroughly.
I noticed some small bugs in the custom magic methods of a few primitives. In particular
Polyline([[0, 0, 0], [0, 0, 1]]) == Polyline([[0, 0, 0], [0, 0, 1], [0, 0, 2])would returnTrue, and a similar bug existed in a few other classes.What type of change is this?
Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.CHANGELOG.mdfile in theUnreleasedsection under the most fitting heading (e.g.Added,Changed,Removed).invoke test).invoke lint).compas.datastructures.Mesh.