-
Notifications
You must be signed in to change notification settings - Fork 56
Updating documentation regarding extended selector syntax #181
Conversation
! or not to invert logic, such as "!(>Z)" to select faces _other_ than the most z facing | ||
|
||
closest to point | ||
support faces, points, or edges closest to a provided point |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a NearestToPoint selector already. Does the current one not fit our needs? https://dcowden.github.io/cadquery/classreference.html?highlight=nearest#cadquery.NearestToPointSelector
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that was the rationale for removing it from the roadmap (i.e. AFAIK it is there already).
%Plane Faces of type plane :py:class:`cadquery.TypeSelector` 0..many | ||
>Y Face farthest in the positive y dir :py:class:`cadquery.DirectionMinMaxSelector` 0..many | ||
<Y Face farthest in the negative y dir :py:class:`cadquery.DirectionMinMaxSelector` 0..many | ||
>Y[-2] 2nd Face farthest in the positive y dir :py:class:`cadquery.DirectionMinMaxSelector` 0..many |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The index on this would be -2 instead of 2? I had assumed that since you're selecting the farthest in the direction, the index would always be positive unless you're wrapping around to the beginning of the collection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this is how I implemented it. So: >Y[-1]
is equivalent to >Y
My mental picture was >Y
is equivalent to sorting, and then the highest element is last.
|
||
box = cadquery.Workplane("XY").box(10,10,10) | ||
.. cq_plot:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cq_plot is probably something that @dcowden added during the documentation refactor, but I'm not sure what it is. It shows up in the rendered documentation for me as cq_plot still.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adam-urbanczyk I tried adding inline comments for the first time, hopefully that works out ok. Thanks for doing this. I think the changes look great, I just had a few questions. |
Thanks @adam-urbanczyk I think this looks good and is ready to merge. Thanks for doing this documentation. |
Related to #179. Do not merge yet.