filter/map/apply/sort/[]/invoke

[voneiden]

Some time back I made cq-filter to allow easier manipulation of workplane objects without breaking out of the fluent API. @adam-urbanczyk requested in voneiden/cq-filter#1 to merge some features from cq-filter into cadquery, namely:

• getitem
• filter
• sort

and optionally

• group

---

First off, I've decided to rename filter to filter_objects and sort to sort_objects. I think the verbosity is warranted as Workplane is a fairly complex object.

The cq-filter implementation of group created an intermediary workplane with extra fields to support slicing groups. I never particularly liked doing it this way, so I spent some time thinking how this could be done without the intermediary workplane. One option, used in this PR, is to create a separate Group object that supports slicing. This does require however the ability for the Group to iterate through candidate objects independently, so it can not be used with filter_objects as it allows the supplied filter function to operate only one one CQObject at a time.

Therefore I needed to introduce another method, currently named as map_objects which hands over the workplane objects to the map function and expects a new list of objects to be returned.

Group supports fixed window groups (x_n - x₀ <= tol) and moving window groups (x_n - x_n-1 <= tol)

---

Currently the PR is missing documentation and tests, but I'm opening this to allow discussion of implementation details. I'm flexible regarding the scope of this PR, if it feels like Group doesn't belong here then we'll drop it.

I'm a bit low on free dev time for the time being so this might progress at a snails pace but we'll get there.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.91%. Comparing base (bf47f12) to head (0c3b810).
Report is 3 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1514      +/-   ##
==========================================
+ Coverage   94.86%   94.91%   +0.05%     
==========================================
  Files          28       28              
  Lines        6228     6259      +31     
  Branches     1262     1271       +9     
==========================================
+ Hits         5908     5941      +33     
+ Misses        193      192       -1     
+ Partials      127      126       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[adam-urbanczyk]

Thanks, it too me a while to get to this. I'll do some rework. I'd like to descope group for now, feels like too big of a change.

[michaelgale]

I like the intent behind cq_filter and the proposed changes. I wonder if these features are in fact already available with the use of the Selector classes. A big clue is the fact that any derived sub-class of Selector must implement their own filter method. In my cq-kit package I extend Selector with a multitude of utility selectors similar to the proposed features such as:
EdgeLengthSelector, WireLengthSelector, RadiusSelector, EdgeCountSelector, SameLengthAsObjectSelector, SharedVerticesWithObjectSelector, and much more

With Selector classes you can effectively filter any CQ object(s) with the added benefit that Selector classes can be usefully combined logically and arithmetically since they implement operator overloading of +, -, and, not, etc.

[adam-urbanczyk]

filter is indeed a quick and dirty way to implement selectors. You'd use it for one-offs and implement a proper cq.Selector for something more reusable. map and apply are for something else.

[adam-urbanczyk]

@jmwright @lorenzncode @voneiden what do you think about it? I'd say this would be it (minus docs and groupby).

I'm still wondering if we should allow to map via Callable[[Iterable[CQObject]], Sequence[CQObject]] too. Regarding groupby it could be implemented using cq.Compound so that no new classes need to be created.

[voneiden]

Looks pretty neat and clean to me, I like it.

Trivial: do you mind if I tidy up the commits a bit? I'd like to amend my original commit message at least since it's very ambiguous.

[adam-urbanczyk]

I'm going to squash anyhows, so why not.

Additional idea: allow callbacks with 0 arity or no rv for debugging (if multimethod allows to dispatch on that). Use case would be quick debugging.

w.apply(breakpoint).apply(show)

[voneiden]

Alright, if gets squashed then I suppose it's fine as is with the surrounding context.

The idea you presented seems like it could be useful, but IMO the current interface is very clean and easy to understand. I'd slightly lean towards having a separate method for this use case but no strong feelings. Multimethod could fine too, if the docstring explains the secondary behaviour in layman terms.

[lorenzncode]

Looks good with minimal testing so far! I'll play with it more.

if we should allow to map via Callable[[Iterable[CQObject]], Sequence[CQObject]] too.

Does this mean something like the following works without an explicit call to compound:

import cadquery as cq
from cadquery.occ_impl.shapes import *


def mycallback(obj):
    if obj.Center().x > 0:
        # return compound([obj.moved((0, -2, 0)), obj.moved((0, 2, 0))])
        return [obj.moved((0, -2, 0)), obj.moved((0, 2, 0))]
    else:
        return obj


r = cq.Workplane().rarray(20, 20, 2, 2).box(1, 1, 1, combine=False).map(mycallback)

[adam-urbanczyk]

if we should allow to map via Callable[[Iterable[CQObject]], Sequence[CQObject]] too.

Does this mean something like the following works without an explicit call to compound:

This would actually imply Callable[[Iterable[CQObject]], Union[CQObject, Sequence[CQObject]] which is slightly more generic. Do you find it useful?

[lorenzncode]

This would actually imply Callable[[Iterable[CQObject]], Union[CQObject, Sequence[CQObject]] which is slightly more generic. Do you find it useful?

If the more generic form can work it might be useful. It looks like there is a workaround for this case if not.

[adam-urbanczyk]

In the meantime I added invoke for calling breakpoint or show inline. It can be also used to implement plugins without monkey patching.

[adam-urbanczyk]

I added some docs and I think this is ready to be merged. Any thoughts @lorenzncode @voneiden @jmwright ?

[lorenzncode]

invoke is another very useful addition! I like the new option for extending CQ. The docs are clear.

Regarding invoke with builtin and arity 0 - my understanding is breakpoint is the primary intended use case (perhaps other special cases).

[voneiden]

Sorry for the slow review, I'm in 🌴 mode. 😃

[voneiden]

Not requesting this to be changed, but I think something like this would be a bit more mundane...?

Suggested change

	if isbuiltin(f):
	arity = 0 # assume 0 arity for builtins; they cannot be introspected
	else:
	arity = f.__code__.co_argcount # NB: this is not understood by mypy
	rv = self
	if arity == 0:
	f() # type: ignore
	elif arity == 1:
	res = f(self) # type: ignore
	if res is not None:
	rv = res
	else:
	raise ValueError("Provided function {f} accepts too many arguemnts")
	return rv
	rv = self
	try:
	res = f(self) # type: ignore
	except TypeError:
	res = f() # type: ignore
	if res is not None:
	rv = res
	return rv

[adam-urbanczyk]

Thanks, but I find mine more clear. Also it has slightly different semantics, f could throw for other reason than wrong number of parameters.

[jmwright]

Looks good to me. Thanks @voneiden !

[adam-urbanczyk]

Merging, thanks @voneiden !