search

voneiden / cq-filter

Manipulate CadQuery Workplane objects fluently
https://pypi.org/project/cq-filter/
Apache License 2.0
3 stars 0 forks source link

Merging with CQ? #1

Open adam-urbanczyk opened 6 months ago

adam-urbanczyk commented 6 months ago

Would you consider opening a PR to CQ with the methods from this repo? If not, anything against adding [part of it] it to CQ?

voneiden commented 6 months ago

Sure, yes either way. What's the [part of it] that you're interested in? The last method is no doubt a wee bit hacky.

Also the IDE specific convenience methods are ugly.

adam-urbanczyk commented 6 months ago

I think the following:

__getitem__
filter
sort

and maybe

group
voneiden commented 6 months ago

Seems good. Maybe the group feature can be refactored to simplify things. At a glance I suppose it might be possible to make a group()[] method that can be used to create a callable that can be passed to filter . Like

wp.filter(group(lambda face: face.Area(), tol=10)[0])

Suppose this would keep the overall API cleaner and remove the need for a temporary variable in the workplane object.

voneiden commented 5 months ago

@adam-urbanczyk fyi I'm slow but I expect to open a draft PR soon. I'm going to suggest having both the original filter and something like filtermap with the former being simple filter expecting Callable[[CQObject], bool] and the latter allowing for more complex use cases expecting Callable[[Iterable[CQObject]], Iterable[CQObject]] .

Grouping could then be implemented without hacking Workplane. There would be a utility Group class that returns a suitable function when indexed (via __getitem__) that can be used with filtermap like so

wp.filtermap(Group(lambda face: face.Area(), tol=10)[0:1])
adam-urbanczyk commented 5 months ago

Sure, filtermap might require some discussion though. I hope that you won't mind.

voneiden commented 5 months ago

No problem at all. Feel free to be as direct with your specs/feedback as you like. If you ultimately don't want Group I'll just yeet it out of the PR. :+1: