Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PoC commit for discussion [filter/map/apply/sort/[]] #1514

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

voneiden
Copy link
Contributor

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 (xn - x0 <= tol) and moving window groups (xn - xn-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.

Copy link

codecov bot commented Jan 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.87%. Comparing base (551a231) to head (73ad16f).
Report is 13 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1514      +/-   ##
==========================================
+ Coverage   94.48%   94.87%   +0.39%     
==========================================
  Files          28       28              
  Lines        5780     6241     +461     
  Branches     1071     1264     +193     
==========================================
+ Hits         5461     5921     +460     
  Misses        193      193              
- Partials      126      127       +1     

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

@adam-urbanczyk
Copy link
Member

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.

Remove group
Rename
More generic []
Add apply / change map
@michaelgale
Copy link

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
Copy link
Member

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 adam-urbanczyk changed the title PoC commit for discussion PoC commit for discussion [filter/map/apply/sort/[]] May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants