docs give examples of tuples where they should be Vectors

[marcus7070]

https://github.com/CadQuery/cadquery/blob/e69b2f83bde43ad6a0159a72bc058165e684e72e/cadquery/selectors.py#L287-L338

The docs suggest DirectionMinMaxSelector((0, 0, 1), True), but this code:

from cadquery import *
test = Workplane("XY").box(1, 1, 1).faces(DirectionMinMaxSelector((0, 0, 1), True))

fails with AttributeError: 'tuple' object has no attribute 'wrapped'.

Changing the tuple to a Vector works:

test = Workplane("XY").box(1, 1, 1).faces(DirectionMinMaxSelector(Vector(0, 0, 1), True))

[jmwright]

Many of the methods will implicitly convert a tuple to a Vector so that either can be passed by the caller. Either we missed that in DirectionMinMaxSelector, or it was dropped in the transition to PythonOCC.

[jmwright]

Here's an example: https://github.com/CadQuery/cadquery/blob/f9d9ae06406aac2feea4a2a94569de76d473a8d0/cadquery/occ_impl/shapes.py#L330

[marcus7070]

Haha @jmwright, you're too quick, I was in the middle of writing the following when you commented.

Rather than change the docs I would suggest we consistently handle tuples or Vectors. But what is the consistent CadQuery way of doing this?

We check type for tuple here: https://github.com/CadQuery/cadquery/blob/e69b2f83bde43ad6a0159a72bc058165e684e72e/cadquery/occ_impl/shapes.py#L481-L501

Request tuple in docs and init a Vector here: https://github.com/CadQuery/cadquery/blob/e69b2f83bde43ad6a0159a72bc058165e684e72e/cadquery/selectors.py#L62-L94

But the Vector class handles being initiated with a Vector, so in the above both a Vector or a tuple would work.

Personally I want to check for the attribute the method needs. eg. DirectionMinMaxSelector needs to dot the vector, so change the init to

def __init__(self, vector, directionMax=True, tolerance=0.0001): 
         self.vector = vector if hasattr(vector, 'dot') else Vector(vector)

edit: NearestToPointSelectoractually fails with a vector, because it does Vector(*self.pnt).

[marcus7070]

Also found https://github.com/CadQuery/cadquery/blob/e69b2f83bde43ad6a0159a72bc058165e684e72e/cadquery/cq.py#L989-L1013

If you follow the code, Workplane.transformed -> convert to tuple -> Plane.rotated -> no check -> Plane.toWorldCoords -> convert to vector.

[jmwright]

@marcus7070 The inconsistent handling of tuples and Vectors is something that it would be nice to fix.

If a Vector can be constructed with either a tuple or a Vector, why add the hasattr checks? It seems like even our type(startVector) == tuple checks are redundant if Vector handles the conversion for us in each case. Is it to protect against the user passing another type of object, or is there another reason that I'm not seeing?

[marcus7070]

@jmwright Yeah, 99% redundant.

Vector init actually grabs an XYZ tuple from the argument and reconstructs a new Vector, so hasattr seems a bit more efficient to me. Also, on the off chance someone extended Vector through a subclass re-initing Vector would clobber it. But these are super minor issues, I would be happy with either method.

https://github.com/CadQuery/cadquery/blob/3cd466a693ce1d7f084e5cd6ddee256fc28d020a/cadquery/occ_impl/geom.py#L11-L51

[jmwright]

Ok, yeah. The hasattr check keeps from assuming too much. I'm ok with doing it that way.

[marcus7070]

Awesome, I might have a go at this when I get some time.

[marcus7070]

I've got an idea that I may as well document here, in case anyone else picks this up before I do.

Throughout CadQuery we accept tuples in the place of Vectors and type check or initialise Vectors in each function. There is a lot of code duplication and different implementations of this. It seems like a worthwhile task to centralise that code into one method we apply everywhere. IMHO a decorator makes sense for this job.

How should devs indicate that a particular argument represents a point in space and can be either a tuple or a Vector? Type hinting seems to make the most sense to me.

Here is a patch that implements it:

diff --git a/cadquery/cq.py b/cadquery/cq.py
index 25dc1d3..7577c4c 100644
--- a/cadquery/cq.py
+++ b/cadquery/cq.py
@@ -19,6 +19,9 @@

 import math
 from copy import copy
+from functools import wraps
+from inspect import signature
+import typing
 from . import (
     Vector,
     Plane,
@@ -33,6 +36,11 @@ from . import (
     exporters,
 )

+Numeric = typing.Union[int, float]
+Tuple2d = typing.Tuple[Numeric, Numeric]
+Tuple3d = typing.Tuple[Numeric, Numeric, Numeric]
+Pnt = typing.Union[Tuple2d, Tuple3d, Vector]
+

 class CQContext(object):
     """
@@ -54,6 +62,25 @@ class CQContext(object):
         self.tolerance = 0.0001  # user specified tolerance

+def tup_to_vec(f):
+    """
+    A decorator that looks for arguments type hinted as Pnts and if they are
+    not Vectors, converts them to Vectors. It is expected that any argument
+    that is not a Vector will be a tuple, but since this is Python we do not
+    type check.
+    """
+    sig = signature(f)
+    @wraps(f)
+    def wraps_f(*args, **kwargs):
+        bound_args = sig.bind(*args, **kwargs)
+        for name, param in sig.parameters.items():
+            val = bound_args.arguments[name]
+            if param.annotation == Pnt and not isinstance(val, Vector):
+                bound_args.arguments[name] = Vector(val)
+        return f(*bound_args.args, **bound_args.kwargs)
+    return wraps_f
+
+
 class CQ(object):
     """
     Provides enhanced functionality for a wrapped CAD primitive.
@@ -856,7 +883,8 @@ class CQ(object):

         return self.each(_rot, False)

-    def rotate(self, axisStartPoint, axisEndPoint, angleDegrees):
+    @tup_to_vec
+    def rotate(self, axisStartPoint: Pnt, axisEndPoint: Pnt, angleDegrees):
         """
         Returns a copy of all of the items on the stack rotated through and angle around the axis
         of rotation.
@@ -873,7 +901,8 @@ class CQ(object):
             [o.rotate(axisStartPoint, axisEndPoint, angleDegrees) for o in self.objects]
         )

-    def mirror(self, mirrorPlane="XY", basePointVector=(0, 0, 0)):
+    @tup_to_vec
+    def mirror(self, mirrorPlane="XY", basePointVector: Pnt = (0, 0, 0)):
         """
         Mirror a single CQ object. This operation is the same as in the FreeCAD PartWB's mirroring

I'm fairly confident I've defined those types incorrectly, but the code works.

Since that decorator inspects arguments, it would be pretty simple to add a try/except around the wrapped function and look for the common mistake of leaving the brackets off a tuple. If the Pnt argument and the one or two after it are all numeric types, append a suggestion to the error message that the user may have left the brackets off a tuple.

Anyhow, I haven't convinced myself that the extra dependencies (and adding partial type hinting) are worth it yet. I'm also not very experienced with type hinting. So I'm not going to make a draft PR yet, I'll just carry on with some other work and think about this for a while.

[marcus7070]

Seems like we want to add type hints to CadQuery anyway: #247