dcowden / cadquery

CadQuery-- a parametric cad script framework
Other
431 stars 56 forks source link

Design Direct API #167

Closed dcowden closed 5 years ago

dcowden commented 7 years ago

CQ 2.0 should have a direct api in addition to a fluent api, for more programmatic use.

jmwright commented 7 years ago

@dcowden What's the definition of a "direct API" in this context? Would PythonOCC be considered a direct API?

dcowden commented 7 years ago

A direct API is still CQ provided, but not tied into the fluent api stack.

As an example, consider this fluent API code:

facesOfCylinder = Workplane('xy').sketch().circle(2.0).extrude(2.0).faces()

The direct api models the steps as strict inputs and outputs, without the fluent stack to chain things. For example, the direct api for that code might look like this:

ctx = ModellingContext()
p = Workplane('xy')
s = Sketch(p)
s.makeCircle(2.0)
resultingCylinder = ExtrudeOperation(ctx).extrude(sketch=s, distance=1.0)
facesOfCylinder = FaceSelector(resultingCylinder).selectAll()

Of course CQ 2.0 would still provide a fluent api-- but the goal would be for that layer to be strictly implemented in terms of the Direct API. Hopefully the result would be much easier to test and read code.

The resulting code will be much more flexible as well. It should be easier to link new operations into the fluent api, and it should be easier to allow the operations to accept input from external sources. As an example, if an extrudeOperation just needs a sketch object and a distance, it doesnt care where the sketch came from at all.

fragmuffin commented 7 years ago

Yes! I have many +1's for this initiative.

dcowden commented 7 years ago

thanks for commenting! Can you elaborate a bit about your use case? I'd like to learn more about how to organize the direct api to make it nice.

jmwright commented 7 years ago

@fragmuffin Thanks for the feedback. I too am interested in your thoughts. On any new features, the more feedback we can get from the community, the better. We're interested in how people would use the direct API, and how they think it should work.

fragmuffin commented 7 years ago

I'm trying to build something very complex, and I would like to do so by

Usually this is given the buzz-term "parametric modeling", but in my opinion the only implementation that allows total freedom is in the form of a language, exactly like cadquery; gui's limit flexibility. I also like the idea of Constructive Solid Geometry (CSG), and cadquery is the best library I've seen that most closely aligns with a capable CAD GUI (FreeCAD) to allow you to verify what you've created (others show you a pretty picture of what you've made, but you can't do much with it)

Just like software development, I'd like to start small, and build up robust, and configuration managed components. Then, to build the entire thing, compile it to output a number of objects:

Modelled objects to classes So I'd like to start this by creating a parts library, and continually add to it.

So for argument's sake, let's say we want to make this pulley into a part that may be used one or a hundred times in a design, each with unique parameters.

Well this is python! simple!, you create a class that holds the design for all pulleys, and each instance of that class is a physical pulley.

But with the current implementation of cadquery, the way to do that would be quite messy, and unintuitive:

import cadquery
from Helpers import show

class Pulley(cadquery.Workplane):
    def __init__(self, radius=20, width=3, wall_height=1, wall_width=1,
                 hole_radius=3.175, key_intrusion=0.92):
        self.radius = float(radius)
        self.width = float(width)
        # ... and so on
        super(Pulley, self).__init__('XY')
        self.build()

    def build(self):
        # note: ignoring the pulley_half concept for now
        self.circle(radius + wall_height).extrude(wall_width) \
            .faces(">Z").workplane() \
            .circle(radius).extrude(width / 2.0)
        # ... and so on, like the linked gist above

    def __copy__(self):
        return self.translate((0, 0, 0))

p = Pulley(width=5)  # a bit thicker than the default
show(p)

but @dcowden 's sample code would yield a much more intuitive and more powerful implementation... which is why I got excited

import cadquery
from Helpers import show
from math import pi

class Pulley(cadquery.Model):
    def __init__(self, radius=20, width=3, wall_height=1, wall_width=1,
                 hole_radius=3.175, key_intrusion=0.92, **kwargs):
        self.radius = float(radius)
        self.width = float(width)
        # ... and so on
        super(Pulley, self).__init__(**kwargs)

    def build(self):
        self.plane = cadquery.Workplane('xy')
        # create keyed hole sketch
        self.hole_sketch = cadquery.Sketch(self.plane)
        # create rolling circumference sketch
        self.rolling_circ_sketch = cadquery.Sketch(self.plane)
        self.rolling_circ_sketch.makeCircle(self.radius)
        # create outer wall sketch
        self.outer_circ_sketch = Sketch(self.plane)
        self.outer_circ_sketch.makeCircle(self.radius + self.wall_height)

        self.extrude(sketch=self.outer_circ_sketch, distance=self.wall_width)
        self.faces(">Z").workplane().extrude(sketch=self.rolling_circ_sketch, distance=self.width)
        self.faces(">Z").workplane().extrude(sketch=self.outer_circ_sketch, distance=self.wall_width)
        self.faces(">Z").workplane().hole(sketch=self.hole_sketch)

p = Pulley(
    width=5,
    origin=(10, 20, 0),  # somewhere else
    rotate=((0, 0, 0), (1, 0, 0), pi/2),  # rotated a bit
)
with open('~/temp/pulley-hole.dwg', 'w') as fh:
    fh.write(p.hole_sketch.dwg_str())
with open('~/temp/pulley-rolling-circumference.dwg', 'w') as fh:
    fh.write(p.rolling_cirk_sketch.dwg_str())
show(p)

I've embellished a bit here; in the above example I'm asuming ExtrudeOperation.extrude function might return a class instance called cadquery.Model (perhaps) And I rushed toward the end, but the idea would be to have all of this behind a library... maybe cqparts:

import cqparts
show(cqparts.Pulley(width=5))
show(cqparts.bolts.Bolt(thread='M6', length=20, type='cup_head'), origin=(10, 20, 30))
show(cqparts.nuts.Nut(thread='M6', origin=(10, 20 15)))

Anyway!, I've rambled a bit here, but I think you get the idea... I'd love to see this library able to go to extremes... build a whole car!, that sort of thing =).

dcowden commented 7 years ago

@fragmuffin have a look at this example from CQ 2.0 and see what you think:

https://github.com/dcowden/cadquery/blob/2_0_branch/examples/direct_api/plate_with_hole.py

The main difference between the simplified examples I gave above and the real CQ 2.0 api is that the direct api contemplates not only performing operations, but performing queries on the result. For example, if you extrude a face into a prism, you probably want to be able to get a reference to the solid and faces that are created, and those that are modified. This is very important because navigating the solid context using selectors is a crucial feature of CQ-- fluent API or not.

fragmuffin commented 7 years ago

I think the biggest thing I want from this is to embrace the Object Orientation (OO) of python.

Using OO with actual objects it should be very clear everybody what's happening (see layer_cake example linked below)

Operation clases & instances in the first block, when creating the rectangle shape, the operation appears to take the role of a factory but only to create a single shape.

Could this code be simplified to this?

#make a rectangle
wire = RectangleWire(
    mincorner=Vector(0, 0, 0),
    maxcorner=Vector(10, 10, 0),
)
rectangle1 = wire.build_shape(name='rect1')  # a list of CreatedShape object
wire.maxcorner += Vector(1, 1, 0) # moves corner out to (11, 11, 0)
rectangle2 = wire.build_shape(name='rect2')

the RectangleWire instance can be re-used, or modified.... I guess this is very similar to what you've written, main difference being, factories usually don't retain references to the things they've created, so they may be created, used, then garbage-collected by whatever requested the instance of the factory.

Remove unclear alterations of models

Something very prevalent in the current API is models changing when it's very unclear in the code.

This example builds a 3 layered circular "cake" (3 layers) gist

import cadquery
from Helpers import show
base = cadquery.Workplane('XY').circle(30).extrude(10)
cake = base.faces(">Z").workplane() \
    .circle(20).extrude(10) \
    .faces(">Z").workplane() \
    .circle(10).extrude(10)
show(base)
show(cake, (200, 200, 200, 0.8))

when displayed, we see that:

obviously this leads to a lot of trial and error when creating something much more complex (before being used to the languages's nuances)

so could this cut code be equally represented as this?

precut_volumes = (created_solids.volume(), created_holes.volume())
drilled_part = Cut_Operation('drill-hole', created_solids, created_holes).perform()
assert (created_solids.volume(), created_holes.volume()) == precut_volumes, "oops: solids were modified, so cannot be reused"
# note: I'm not implying the `volume()` method exists, it's just to illustrate what I mean

or perhaps

drilled_part = created_solids.cut(created_holes)

# to alter te original
created_solids = created_solids.cut(created_holes)
# or if duplicating parts to cut away is too resource hungry, equivalent:
created_solids.cut(created_holes, change_base=True)  # returns self

Helping?

Is this helping?, or am I just complaining (you can tell me if I'm complaining :stuck_out_tongue_winking_eye: )

dcowden commented 7 years ago

Thanks for this feedback!

Yes, with the fluent api, there are necessarily some assumptions and state changes... I think this is unavoidable in this type of API. In some cases, you want the new variable to be updated to reflect your modifications, and sometimes you don't. The think the current fluent api is confusing because it is not explicit enough about the user's selection of the scope of each operation, as it relates to the modelling context.

For example, suppose you do this:

base = Workplane('xy').circle(20).extrude(1) top_of_base = base.workplane().circle(10).extrude(1)

You could plausibly want top_of_base to be only the new layer, OR you could want it to refer to the unioned result. You could also conceivably want the base variable to refer to only the original base, as it was before the creation, or the unioned result. cadquery does what many cad packages do by default-- combining things assuming you don't want to deal with all of those intermediate copies, and eliminates the need to manually fuse all of the items together. This is probably what you wanted to do nearly all of the time. But this implicit behavior, which is confusing.

Making the api support both possible ways of working becomes quite a challenge. The current api does it by providing the 'combine=True|False' option, that allows the user to select which-- but its easy to miss.

Regarding the factory, yes, currently operations are designed as single use factories, not re usable ones. This is primarily because the operation name itself is later used as a way to select features by the name of the operation that created them, and the resulting metadata is extremely nontrivial. If we want a stateless factory here, we would need to return not just a list of created objects, but also those which were deleted, and those which were modified. You could argue that the success and errors could raise an exception.

I suppose an API like that is possible, but it becomes more like returning back result object not a simple list of shapes.

fragmuffin commented 7 years ago

That's ok!, no problem.

I'm looking at the implementation from the top down, so I'm typically going to ask for magic :wink: .

I've started a cqparts library, it's just a concept at the moment, and I don't know how far it'll go, but it's shiny to me at the moment :sparkles:

dcowden commented 7 years ago

I certainly appreciate it! More views of what would be perfect is precisely what we need!

I would be especially interested in 'real' objects you are making and the associated code and pseudocode. I certainly agree that the power lies in a huge library of parts that can be re used.

One question you might contemplate along those lines is how different modules should interface when it comes to references to other geometry.

For example, it's easy to imagine how a pulley object works... But how do we control where in 3d space it is created? Essentially we end up with the program equivalent of solidworks mating relationships... Likely, I want to be able to drop in this object and have it's azis aligned with another axis in the model.

Another example is how to deal with changes to existing objects. For example, suppose I have a module that does counter bored holes... I need to accept some points, and, most importantly, a solid object to drill into. The target object will be modified, which means:

(1) the user will probably expect that references to the original object now refer to the changes object. Is that what we want?

(2) how do we accept a reference to the solid we want to modify, without exposing details that are specific to our underlying implementation? I, we don't want people passing in an OCC solid or a freecad solid

For the notion of re-usable code to work at scale, the interface design is key.

What are your thoughts?

On Aug 28, 2017 7:38 AM, "Peter Boin" notifications@github.com wrote:

That's ok!, no problem.

I'm looking at the implementation from the top down, so I'm typically going to ask for magic 😉 .

I've started a cqparts https://github.com/fragmuffin/cqparts library, it's just a concept at the moment, and I don't know how far it'll go, but it's shiny to me at the moment ✨

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dcowden/cadquery/issues/167#issuecomment-325330158, or mute the thread https://github.com/notifications/unsubscribe-auth/ABPOAzzfpJU1wKDZSMnbOU13sjQ0A7Slks5scqa-gaJpZM4LKCOZ .

fragmuffin commented 7 years ago

@dcowden Another example could be a plate with a screw or bolt through it...

Operations instantiated from parts The pulley would have an associated axis with its definition (like you said, to align with another). But the bolt may have an associated cut operation instance, ready to be applied to the plate. That way, if you wanted to replace a bolt with a countersunk screw, the cut shape would change. Similarly, whatever the plate is being bolted to would have a smaller hole, for the screw thread to bite into... So a screw would actually have 2 cut operation instances.

Other types It's difficult to tell what may be needed, but perhaps that's up to the author of each part... The better a part is made, the more usable it is.

Parts Library I'm writing cqparts as a parts library basis (abstract) that can be used for others to create their own library... perhaps even proprietary parts. (but also because I don't want to maintain a parts library for a billion boring nuts and bolts 😉 ) It's best explained in the readme

(1) the user will probably expect that references to the original object now refer to the changes object. Is that what we want?

I think the main thing to aim for is clarity. I'd expect that with a library like this, the biggest factor for it being adopted is usability, and usability is subjectively determined by the user. So ultimately, of the user expects it, then that's what they should get... But that can also be changed by the wording of the API... For example.

part1 = union(cube, sphere)
part2 = copy(cube)
part2.add_union(sphere)
# part1 == part2

Also: what are your thoughts on operator overriding?

part1 = cube | sphere
part2 = copy(cube)
part2 |= sphere
dcowden commented 7 years ago

I'm not a fan of operator overriding for this use case. The number of things times you perform the very basic boolean operations is small, but the readability impact or a novice is large. It doesn't seem worth it to me.

That said it's also a decision that can be deferred. Clearly even if it is supported, it would be done in terms of more verbose functions, so adding the overloads as shorcuts would trivial

dcowden commented 7 years ago

@fragmuffin in your above code, if we were to add this code after line 1:

part3= cut_hole(part1,....)

Part3 clearly is part 1 but with some kind of hole in it. But what is part1? Was it modified by side affect, or was a copy implicitly created?

Or are you suggesting there would be both make_hole and make_hole_with_copy?

Today, cq assumes in this case that you want to modify the original, and that you want part1 to refer to the same object as part3 in this case.

fragmuffin commented 7 years ago

Fair enough with operator overriding.

I think I would assume (write or wrong) that part3 = copy(part1), and then part3 is modified with the cut operation.

# create new part with part1 as the template
part3 = cut_hole(part1, hole_obj)
# functionally the same...
part3 = copy(part1) # currently: part1.translate((0, 0, 0))
part3.cut_hole(hole_obj)

make_hole and make_hole_with_copy?... no, that would have to be done for every operation, very messy...

I can see why self needs to be returned by most class functions, because the vision of this cadquery is to make coding geometric parts similar to the way you'd describe them.

you know what!... I feel like all of this would be made simpler if the __copy__ was implemented for CQ objects.

def __copy__(self):
    return self.translate((0, 0, 0))

I've just been playing around with:

import cadquery as cq

def copy(obj):
    # should be: from copy import copy
    return obj.translate((0, 0, 0))

template = cq.Workplane('XY').box(50, 50, 2, )
bolt = cq.Workplane('XY', origin=(0,0,-10)).circle(1.5).extrude(20)

part1 = copy(template)
part1.cut(bolt)
part2 = copy(template)
part2.cut(bolt.translate((10, 10, 0)))
# it works!

both parts have a single hole in them... but I can only do this if I explicitly create a copy of the template object first.

dcowden commented 7 years ago

The copy idea is a good one!

So just to make sure I understand, this code ( a very slight change to yours above) would create a part with two holes in it:

def copy(obj):
    # should be: from copy import copy
    return obj.translate((0, 0, 0))

template = cq.Workplane('XY').box(50, 50, 2, )
bolt = cq.Workplane('XY', origin=(0,0,-10)).circle(1.5).extrude(20)

part1 = copy(template)
part1.cut(bolt)
part1.cut(bolt.translate((10, 10, 0)))

is that right?

fragmuffin commented 7 years ago

yes, it does, and that's what I'd expect.

dcowden commented 7 years ago

@fragmuffin ok thanks I understand the semantics you expect, this has been quite helpful.

It's non trivial to achieve the semantics above in complex cases. The main reason is that in the modelling layer, most operations produce new objects, rather than the original. Cadquery currently uses some trickery to ensure that old references to the updated geometry as you expect in this last case. It can be even more complex than it appears, because objects previously defined can sometimes need to be changed, even when they were not part of the current operation.

It's done in cadquery today through the use of a shared reference to a single modelling context.

Consider this example:

template = cq.Workplane('XY').box(50, 50, 2, )
bolt = cq.Workplane('XY', origin=(0,0,-10)).circle(1.5).extrude(20)
top_face=template.faces(">Z")
part1.cut(bolt)
part1.cut(bolt.translate((10, 10, 0)))

Conceptually, top_face refers to a face who's geometry changed in the cut operation. Technically, the two objects are completely separate. There are many other cases in which the top_face object ceased to exist entirely.

The philosophical question i've been struggling with is whether dealing with these types of issues belong in the direct api or not. I think they do, because even when you use the direct api, you still care about how this stuff works. Handling the reference issues like these is why the operations in the direct api cannot be completely stateless.

fragmuffin commented 7 years ago

@dcowden As I write these opinions I'm still learning the language... I feel I'm only beginning to understand the complexities you describe, which gives me new appreciation for the cadquery and OpenCascade framework.

As for your philosophical question, I don't know which way you should go... I do like transparency, but not at the cost of usability; if an API is too complicated, I find myself wondering if I need it at all (in this case, opting to just use OpenCASCADE directly).

Semi-related: I'm using the Tower Pro MG90S micro servo as a bit of a case-study for the cqparts lib I'm toying with at the moment... so with that experience, I may have something of more value to say soon ;)

dcowden commented 7 years ago

@fragmuffin understood, thanks! I'm enjoying the interchange, its actually really useful to have your perspective. Mine is permanently confused and influenced by the details.

I've also done a lot of development with Onshape Featurescript, an odd proprietary language that allows automating things in OnShape.

Though i'm not overall a huge fan of FeatureScript, they do have an opinionated approach to this problem. In Featurescript, you're essentially not allowed to have variables that refer to model geometry. When you want to refer to a model thing ( a face, a solid, an edge), you are required to build what's called a query, which runs and selects geometry. Its basically a selection function.

Operations never accept faces, solids, or other geometry-- they always accept queries. Pseudocode would look like:

cq.Workplane('XY').box(name='box' 50, 50, 2, )
cq.Workplane('XY', origin=(0,0,-10)).circle(1.5).extrude(name='bolt',20)
box=query(createdBy='box', type='Solid') #select all solids created by the box operation 
bolt= query(createdBy='bolt', type='Solid') #select all solids created by the bolt operation
cut(box, bolt)

Because the cut operation accepts queries, not actual object references, and because the box and bolt variables are queries, not actual objects, we avoid the confusion of what gets changed. Since you cannot ever have a variable that stores a face or other geometric item that can be out of date, its very clear what happens every time. The other advantage is that because all operations need queries as input, it tends to mean that the ability to run queries to select geometry is front-and-center. That's good, because as you make complex objects, the ability to select features on existing objects becomes key.

The negative ( a HUGE one) is that its extremely confusing and hard to learn and use. Its extremely non-intuitive to figure out how to select the geometry you just made in order to do the next step. But once you do it, the operations themselves are much easier to write.

It would be nice to get the benefits of both approaches, but I've not figured out how to do that.

fragmuffin commented 7 years ago

@dcowden I'm glad I can help... I understand what it's like to design something on your own, having somebody to bounce ideas off is always helpful.

Query vs Instance, like Database ORM's? What you describe sounds a lot like an ORM used by any scaleable web framework. I'm most familiar with django.

In django a Model is a class-based definition of a database table (a crude explanation, but it'll do for now). A query is built into a QuerySet instance that knows everything needed to create an SQL query (or potentially other languages) to send to the database server. Instantiating a QuerySet does not automatically touch the database, rather iterating through it (among other commands) causes the fetch of database results.

>>> Person.objects.filter(name='bob')  # returns a QuerySet
>>> print(Person.objects.filter(name='bob').query)
SELECT "people_person"."id", "people_person"."name", "people_person"."age" FROM "people_person" WHERE "people_person"."name" = bob

>>> # The database has not been queried yet... the following send SQL to the db service
>>> Person.objects.filter(name='bob').first()  # returns an instance of Person (if anybody named 'bob' exists)
<Person: bob>
>>> Person.objects.filter(name='bob').count()  # changes underlying query to exploit SQL's COUNT function before the query is sent.
42

>>> # Writing to the database
>>> p = Person(name='billy')
>>> p.save()  # there are more elegant ways to do this, but again, this is just illustrative

The distinction between a QuerySet and an iterable list of Model instances can be confusing to beginners, but when you get used to it, the distinction is not only easy to identify, but can be powerfully exploited. This way you get both; the query pointing to the object, and the object itself.

Have you used django (or something similar)?

Encouraging Bad Code? Similarly to django, this may encourage developers to simply work with object instances over their queries because it's conceptually simpler, but I'd argue that that's not always a bad thing. Something that comes into existence, beyond the inception phase (that may not have existed otherwise) can always be optimised by a more experienced developer. It encourages growth.

dcowden commented 7 years ago

@fragmuffin Yes I'm reasonably familiar with Django, I have used it for a project or two

You are right that this is a similar problem. The analogy holds well for queries, but not as well for models.

I'd argue that the relationship we have here is more like database and SQL itself. On the query side it's just as you have described. But on the update side, it's more like directly running an update statement. You can't really cleanly just update a single instance. You can perform operations, which could affect one or more rows, and to see what happened you have to run another query.

I'm going to refresh myself on Django orm, and then I will post some proposals for how a direct API might work, which combines the thoughts in this thread and what I have already.

dcowden commented 7 years ago

@fragmuffin @jmwright This is definitely not anything more than a start, but see what you think about this document:

https://github.com/dcowden/cadquery/blob/2_0_occ/doc/directapi.md

Here, i've tried to give some example code and concepts to keep the discussion going.
I've decided that the api layer exposed to the user ( which would include both a direct api and a fluent api) cannot be the same as the lower level Operation interface. This is mainly driven by the need to handle references to objects that could be changing via the use of queries.

The way of working may appear to be overdesigned, but it think the layers are necessary because the underlying CAD system always creates copies of objects when they are changed. IE-- at the CAD kernel layer, there is no such thing as a 'modified' object. There are only new ones. In order to allow a user to store a reference to a shape with is later modified, but still have it point to the same shape, we must create a cross-reference to update all of the references in memory. This magic is done with the query layer, through the use of a shared context ( which can track all of the objects created)

Let me know what you think!

fragmuffin commented 7 years ago

@dcowden I like it!, just a few questions

Queries & Evaluation

At the end of the file, would the following be true?

assert half_box.modified().solids() is created_solids
assert created_solids.evaluate() != list_of_solids

if so, perhaps the list should actually be a set, since I don't think the order the solids are listed would matter, and the same solid can't be in the list twice.

Custom Operations

Can users of the API make their own operations? eg:

# with a function
def cut_rect_thru(dapi, solids_query, length, width):
    # cut keyhole through object's center along Z axis
    for solid in faces_query.evaluate():
        tool = dapi.box(solid.center, length, width, 999)
        solid = dapi.subtract(solid, tool, update=True)
        solid.cut(tool)

# or inheritance
class MyAPI(cadquery.DirectAPI):
    def cut_rect_thru(self, solids_query, length, width):
        pass # as above

# or by registration
@cadquery.operation("my_op")
class MyOperation(cadquery.Operation):
    def __call__(self, solid, p1, p2):
        pass  # do stuff?

solid1 = dapi.my_op(solid2, param1, param2)  # or whatever

I'm toying with the idea of a list of operations that can embed an appropriate screw drive pattern into a face... similar to cskHole but with specific shapes. At the moment it's very crude, I think I'll wait for this API before releasing cqparts

dcowden commented 7 years ago

@fragmuffin thanks for the comments. You are right about the ability to do custom operations. its very important. In my experience , re-usable operations are actually far more valuable than re-usable parts. I definitely need to give more thought to how new operations are added, so that its very easy and clear.

The main value of the library will eventually come from a large collection of shared operations ( boss, rib, CSK hole, imperial/metric sandard holes and taps, mold cavities, flattened sheet metal, screw threads, etc. It needs to be very easy to collect these, and, going a step further, include ones others have written into your installation.

So we need to support two use cases: (1) as a user, I can author my own operation, which can be added into the direct api, (2) as a user, i can include custom operations from other users without modifying my source code

dcowden commented 7 years ago

@fragmuffin yes i think the statements you have listed would be true. In both cases, you might get a list of objects, and the entries may be the same, but the list object itself would be different, since it will be newly created. ( IE, evaluate() will always create a new list, but if no change have happened, the contents will be the same )

Still thinkin/working on custom operations

dcowden commented 7 years ago

@fragmuffin @jmwright what plugin mechanism have you seen done in python that you like the best?

i think custom operations need to be easy to add-- even when the source is not yours. I feel like i just want a user to be able to download a pckage that has some operations or point to a url, and have them 'just available'. In Java land, we'd do this by using annotations and then scanning for classes that advertise that they are plugins, but i'm not familiar with how to do that in python. I guess thats the last option in @fragmuffin 's suggestions-- but how can you do that when the source is loaded via url or something? seems like something like this would be ideal:

import cadquery as cq
cq.import_operation('https://path/to/some/operation')
#now operations magially work as if they were direct
adam-urbanczyk commented 7 years ago

@dcowden

Some comments/thoughts on this topic:

Some pseudocode to illustrate:

import cq.dapi as cq

ctx = cq.context()

s1 = ctx.box(1,1,1) #s1 is a Solid

with cq.Sketch(s1.faces('>Z')) as sketch:  # upon exiting the with block constraints are solved
    c1 = sketch.circle(0.5)
    c2 = sketch.circle(0.1)
    sketch.add_constraint(c1,sketch.parent.vertices('>X and >Y'),'coincide')

s2 = ctx.cut_through_all(sketch)  #s2 is a new Solid
s3 = ctx.difference(s1,s2) #I can make yet another solid by e.g. performing subtraction
jmwright commented 7 years ago

I think it is also essential to not forget about current users and not make any changes to the current (fluent) API

This is an important issue for me. Introducing major breaking changes in the wrong way has caused a lot of problems in other open source projects. I think the idea is to keep CQ 1.0 as-is, while still maintaining it and maybe backporting useful features from CQ 2.0 into it. I'm nervous about splitting the project like that, but CQ 2.0 is probably going to be too different from CQ 1.0 to keep everything on one silo.

fragmuffin commented 7 years ago

@dcowden I think my favourite mechanism for expansion is registration; the decorator example in my last comment.

an expanded version of that:

#!/usr/bin/python
import six
from collections import defaultdict

# ================= LIBRARY CODE =================
# ---- Registration
registered_operations = defaultdict(dict)
def register(obj_type, name):
    assert issubclass(obj_type, ObjectType), "bad object type: %r" % obj_type
    assert isinstance(name, six.string_types), "bad name: %r" % name
    def inner(cls):
        assert issubclass(cls, Operation), "operation classes must inherit from Operation"
        assert name not in registered_operations, "duplicate operation name '%s': %r:%r" % (
            name, cls, registered_operations[name]
        )
        registered_operations[obj_type][name] = cls
        # option to change class before it's returned.
        # futureproofing:
        # good if you want to make class changes in future versions of cadquery
        return cls
    return inner

# ---- Operation (prototype)
class Operation(object):
    def __init__(self, obj):
        self.obj = obj

    def __call__(self, *largs, **kwargs):
        return self.evaluate(*largs, **kwargs)

    def evaluate(self, *largs, **kwargs):
        raise NotImplemented("evaluate not overridden by %r" % self.__class__)

# ---- Objects (shapes, edges, faces, etc)
class ObjectType(object):
    def __getattr__(self, key):
        if key in registered_operations[self.__class__]:
            op = registered_operations[self.__class__][key](self)
            return op

        if key in self.__dict__:
            return self.__dict__[key]

        raise AttributeError("'{cls}' object has no attribute '{key}'".format(
            cls=self.__class__.__name__,
            key=key
        ))

class Shape(ObjectType):
    pass

# ================= USER CODE =================
@register(Shape, 'do_stuff')
class DoStuff(Operation):
    def evaluate(self, value):
        # self.obj will always be a Shape instance
        print("doing stuff to %r, with a value of %s" % (self.obj, value))

s = Shape()
s.do_stuff(9001)

output:

doing stuff to <__main__.Shape object at 0x7f77b27ed790>, with a value of 9001
fragmuffin commented 7 years ago

@dcowden regarding your concern with the imported libraries, as long as the downloaded libraries are import ed, or rather, executed before the build code is executed, it should be fine.

I think that's a python nuance; out of scope for cadquery :wink:

good

import cadquery as cq
cq.import_operation('https://path/to/some/operation')

def make_a_thing():
    pass # create shapes and perform operations on them

def make_another_thing():
    pass # create shapes and perform operations on them

make_a_thing()
make_another_thing()

bad

```python
import cadquery as cq

def make_a_thing():
    pass # create shapes and perform operations on them

def make_another_thing():
    cq.import_operation('https://path/to/some/operation')
    pass # create shapes and perform operations on them

make_a_thing()
make_another_thing()

the latter may cause issues if the functions are called in a different order, or the libraries are changed

Peque commented 7 years ago

I agree with @jmwright, CQ 2 is maybe too different, so braking changes are probably required. I think it would be for the best. CadQuery is still a moderately used/known package so this could be a good opportunity to make it better and gain more users.

@dcowden Not really related to the API design but, if this is going to be a major rewrite, would you consider using pytest for the tests? I could help for the transition/porting if necessary. In my opinion makes things easier/cleaner and is probably more appealing for new contributors (maybe that is just my impression :sweat_smile:).

dcowden commented 7 years ago

@all, certainly reverse compatibility is important, but i'm not worried about it now. I'm imagining a fluent api that works very similar to cq 1.x, but that's implemented on top of some of the api's we are discussing now. I think we'll be able to save compatibility. CQ 1.x pretty much ONLY has the fluent api, so we can preserve it while developing new apis beneath.

@Peque pytest is no problem-- that's a move i had already planned ( well, either pytest or nose).

@fragmuffin, I agree, i think you'd want to import them. as you put at the top. I'm not sure how best to combine this approach with your earlier registration example. that said, for now it's probably an implementation detail we can work out when we have some other things nailed down.

@adam-urbanczyk , thanks for your comments. I like the with block concept on Sketchs-- very nice, i'll include that for sure.

Consider this code:

   s1 = cq.box(1,2,3)    # a box operation, results in a box (s1)
  faces_s1 = s1.faces(">Z") refers to a rectangular face
   s2 = cq.cylinder(1.0,20.)  #s2 is a cylinder, and this was a cylinder operation
   s3 = s2.subtract(s1)  #s3 is a box with a hole in it. this was a subtract operation

There are two key questions regarding even this simple code:

  1. What does s1 and faces_s1 point to after the subtraction?
    1a. Exactly what they pointed to before s3 was created, which implies that s3 is a new object. In this case, every operation will create a new set of objects in memory. it also means that its kind of 'weird' if you use s1 again later-- because its a copy. This is not compatible with cq 1.x behavior, but that can be corrected quite easily by an adapter layer. This approach is much simpler to implement, but leads to garbage collection problems I think.

    1b. s1 points to the same object as s3. This is how most CAD systems work, and how cq 1.x works. When you subtract one object from another you don't get a copy of the original, you usually get the modified result on the screen.

  1. How do we select faces that were created/modified by an operation? For example, in this case, I'd like to select the face created by the subtraction-- which will be the inner cylindrical face of the box. But how do i do that?

    2a. s3 could be a result of the operation, rather than directly a solid. This way, its possible to get the created/modified items . This is what I have proposed for CQ 2.0. The idea is that each operation has an auto-generated ID, and the result of the operation allows you to get at the result and questions about the result. This is more like CQ 1.x, because in CQ 1.x you get a CQ object, not a solid directly.

    2b. s3 could be a solid, but then i need to have some other contstruct to query the faces created by the operation? An alternative would be to allow the user to supply an operation id on each operation, and then use selectors to query for example:

      s3 = s2.subtract(id='hole-punch',s1)  #s3 is a box with a hole in it. this was a subtract operation
      hole = cq.created_by('hole-punch').created.faces()

Regarding question 1, 1a is not how CQ 1.x works, but its simplicity is growing on me. It seems wasteful, but i'm sure we can solve that somehow. It will impact how some operations work. For example, a split operation that splits a solid by a plane must return two solids, because the original solid will not be modified. In fact, with option 1a, any topological object created is by definition immutable.

Regarding question 2, I do not believe that having direct API methods return topology objects directly (2b) is a good idea, because it is too limiting. When you perform an operation, the resulting solid(s) that were created is only one possible question you need to answer (though i certainly agree it is the most common). Sometimes you want to know other details. Queries are one way to solve it, but there are others as well. When you return a topology object, (a) the caller must know what type of object to expect, which can result in nasty code to detect solids vs compounds and such., and (b) the caller doesnt have a good way to learn what faces/solids were created, modified, deleted, etc.

So i think i'm leaning towards 1a and 2a.

Thoughts everyone?

dcowden commented 7 years ago

Hi @fragmuffin @Peque @adam-urbanczyk @jmwright

I have posted another version of the direct api example-- this time with a picture and some better examples, incorporating your feedback.

Let me know your thoughts on this version please

https://github.com/dcowden/cadquery/blob/2_0_occ/doc/directapi.md

fragmuffin commented 7 years ago

@dcowden I agree with your preferrence for 1a.

However, I'm tending more toward 2b for the operations. but perhaps an approach like this instead:

s3 = s2.subtract(s1)  # s3 is a box with a hole in it
# alternatively, the user can run
(s3, hole) = s2.subtract(s1, ret=('result', 'faces'))  # s3 is a box with a hole in it (as above). hole are the resulting faces

so ret is ('result',) by default. if len(ret) > 1 then a tuple is returned

this way you could have the best of both 2a and 2b

so perhaps each evaluate() function of an operation should return a dict which is expected to have a 'result' key, and any number of optional extras. for example, subtract could also return a callable that would return the s1.intersect(s2), because that's the difference between the old solid, and the new one.

get_diff = s2.subtract(s1, ret=('diff_query',))
diff = get_diff()  # a solid representing the chunk to be removed from s2
fragmuffin commented 7 years ago

that also solves the garbage collection issue of retaining faces data for every cut done in the model, on the off chance that the user will request the result later... If the data is returned in dict, but not used, it should be garbage collected almost straight away.

dcowden commented 7 years ago

@fragmuffin good idea about the dict. What are your thoughts regarding my latest API example? I changed it after posting my last comment. In that example, I ended up returning a selector, which accomplishes the syntax most people probably want ( to get faces and such off the result), while still allowing access to other more complex queries.

I was stuck for a while till I realized that the user expects even a basic 'solid' object to support a simple faces or edges method. Returning s selector accomplishes that, plus gives access to many other possible selections.

Thus , after box= cq.box(1,2,3)

We can have

box.faces()

Or

box.faces('modified')

fragmuffin commented 7 years ago

I think it's good.

But I still think the tangential data that comes from an operation (ie: the created and modified elements explored in the chamfer hole example) should be requested at the operation.

So with the registration example above it would look something like.

# ... code removed defining (ctx, box, sketch)

# cut a hole using the sketch
(box_with_hole, created, modified) = ctx.cut_thru_all(
    sketch, ret=('result', 'created', 'modified'),
)

# at this point, the old solid is still in the tree, referended by box
# the new solid has 8 faces, one of which is cylindrical
assert len(box.faces()) == 6
assert len(box_with_hole.faces()) == 8

# in the new solid, 1 face was created, 2 were modified, the others are the same as they were
assert len(created.faces) == 1
assert len(modified.faces) == 2

I'm sure this could be cleaned up.

Proof of Registration Concept Proof of concept in this gist with the added ability to specify different ret values

There you can see that the modified result is only processed if it's requested at the time of the operation's execution, so it doesn't have to be processed, and added to a stack, even if it's never used (and the same for created)

Operation Inheritance I think operations should also be able to be inherited. For example, a counter_sunk_hole operation could inherit from hole, execute it's super(<cskHole class>, self).evaluate(*kwargs) then continue to add the chamfer.

so you'd have something like:

class HoleOp(BaseOperation):
    def evaluate(self, diameter):
        pass  # code here

class CountersunkHoleOp(HoleOp):
    pass  # do the thing

class CounterBoreHoleOp(HoleOp):
    pass  # do the thing

Anything you choose, I'm excited.

dcowden commented 7 years ago

I think the syntax you describe would work. The main concern I have is associated with another use case I have in my mind, but have not described. Let me explain that.

Each operation has and ID, and logs what objects it created, modified, and removed. I'm planning to store the result of all of the operations in the context. The goal is to make it possible to select features that were created/generated by any operation at any point in time.

In my current thinking, the variable returned by an operation is a short-cut to get a selector that filters the objects to those performed by the current operation.

I would like it to be possible to select objects anywhere in the context at any point, by using solids, faces, or operation ids. To do this, I'm magining that its possible to build selectors that are not connected to a particular operation-- a stand-alone selector, if you will, like this:

solids = ctx.select_solids(operation_id='my_hole')

When I was contemplating this use case, I found that it would be best to focus selection on selectors, rather than operation methods, so that the way of doing it is the same.

I'm also thinking about extenstion, and if the operation accepts ret='' as a way to control the seleted result, it means that any add-in operations must declare this parameter-- but they could easily leave it off accidently, leading to an inconsistent interface.

Consequently, I was seeking a way to 'factor out' the selection stuff so that its not something that operations have to deal with.

fragmuffin commented 7 years ago

@dcowden OK, I see what you want. The returned dict could simply be added to another dict contained within ctx with operation_id's as the key

Could the operation stack be optional? The option to access the results for all prior operations may eat too much memory. Q: Would operations only be remembered if they were explicitly given an operation_id?

Case Study: Involute Gear This webpage describes a universal method on creating an involute gear tooth profile: practical part geometries (see section 6.2.5) It introduces the concept of incrementally cutting the tooth profile out of a cylinder, then duplicating it in a circular array.

The tool cutting visualisation: tool cutting visualisation

I only show this to illustrate how many complex operations may be used to make a single simple part.

ret & operation_id parameters

if the operation accepts ret='' as a way to control the seleted result, it means that any add-in operations must declare this parameter

that's not true; look at the DoStuff(Operation) class declared in the proof of concept... and how it's referenced in the examples. This could also be true for an operation_id parameter; the parameter could be stripped off like the ret parameter is so it can be added to any operation, as long as it inherits Operation.

jmwright commented 7 years ago

Thanks to everyone who's gotten involved in this discussion. It's encouraging to get feedback from the community, and it helps determine the design and future direction of CadQuery.

From comments by @adam-urbanczyk :

I do not like the idea of queries - for me it seems like a lot of abstraction with no clear benefit/purpose. I'd be much more happy with the current string based selector syntax

I'd be much more happy with the current string based selector syntax

I'm a huge fan of simplicity and I do like the string selectors. My hope is that we'll be able to maintain simplicity while still getting the needed functionality. As @dcowden has mentioned, it's expected that any complexity that is necessary in the direct API can be dealt with in the design and implementation of the fluent API that will be on top of the direct API. There's no guarantee, but that's the expectation.

It would be nice to make an explicit Sketch class that can be reused (e.g. mapped to different faces) and eventually will support constraints

@dcowden +1'd this and so do I. That would be a nice feature.

From a comment by @Peque

...would you consider using pytest for the tests?

Sounds like @dcowden is ok with this. Is it something we should create an issue for and start working through in the current repo?

General thoughts:

Peque commented 7 years ago

jmwright: I really, really like the idea of operations and parts that can be pulled in from places like a community member's repo [...] have had problems where someone will delete their popular project/repo and it breaks thousands of other projects [...]

I would not go for a URL-based import here. Why not using PyPI instead? Easy to handle dependencies, versioning (important to avoid breaking others code as well)... And it is not hard to configure.

jmwright: [...] I'm going to start on CQ 2.0 support. [...]

Agree. This would be the easiest way for users to try the new API, see how it feels like and give some feedback. For those of us who do not have a lot of experience in CAD languages the best way to understand how it feels is to try it out with our own designs to see if there is something missing/desirable.

jmwright: Sounds like @dcowden is ok with this. Is it something we should create an issue for and start working through in the current repo?

Maybe start working through in the current repo means a lot of work (rewriting every test takes some time). Would not it be easier to do it with CQ 2.0 considering it will be a complete rewrite and the code base will grow "from zero"?

dcowden commented 6 years ago

@Peque

I would not go for a URL-based import here. Why not using PyPI instead? Easy to handle dependencies, versioning (important to avoid breaking others code as well)... And it is not hard to configure.

The primary motivation of the download idea is to eliminate the overhead needed to create a package to share a module. Its quite a lot of work to make a setup.py, set up a CI build, and get a pypi account, just to make and upload a package. I fear that if we make it that hard, we'l miss out on a lot of contributions from users. golang benefits from HUGE growth as a result of ability to import github projects directly. If users are worried about the stability of the code, they can always just copy the source into a local module.

@fragmuffin

Could the operation stack be optional? The option to access the results for all prior operations may eat too much memory.

Definitely. I agree with this concern. users should probably opt-in to tracking everything, because its probably rare that people need to do it.

adam-urbanczyk commented 6 years ago

@dcowden

On your two questions:

1) As far as I am concerned definitely 1a. I'd prefer no magic happening in the background.

2) I'd propose that solids do not own the operations, but rather the context:

s3 =  ctx.cut(s1,s2,id='hole')
faces = ctx.find('hole').created().faces()
faces = ctx.find(s3).modified().faces()
#etc

The modified proposal is clear, but I have one question: why the following

return Query(self.ctx).filter(operationId=operation_id)

i.s.o. for example this

return self.ctx.filter(operationId=operation_id)

Do you want to make the Query lazy? That does not require using a separate class, ctx,filter could return a generator if it is beneficial (I am not sure it is by the way).

dcowden commented 6 years ago

@adam-urbanczyk thanks

I think the second example you have is better probably. I think when i wrote that i was thinking more about the operationID part and not thinking carefully about the first part.

I'd propose that solids do not own the operations, but rather the context

By 'operations', do you consider only operations by the definition I shared ( a process that modifies one or more shapes), or are you referring more generally also to methods that include selectors?

One big design question is where selector methods live. In CQ 1.x, many of the methods are actually on the shape objects. This is mainly because this is/was how it was done in FreeCAD. For example:

https://github.com/dcowden/cadquery/blob/master/cadquery/freecad_impl/shapes.py#L296

In the fluent api, of course the fluent objects have wrappers for these selectors. But in the direct api, there is a question about where these methods are implemented.

My thinking is that the functionality must be actually implemented in selector objects, much like is done today in CQ 1.x. Given that's the case, it means that selector methods on the context ( as you have illustrated above) or on an intermediate result object ( as I proposed in the example), probably both delegate to an implementation elsewhere anyway.

Considering your code again:

  s3 =  ctx.cut(s1,s2,id='hole')
  faces = ctx.find('hole').created().faces()
  faces = ctx.find(s3).modified().faces()

An interesting question is what methods are available on s3. I think that after this:

  s3 =  ctx.cut(s1,s2,id='hole')

That these two lines should produce the same result:

faces = ctx.find('hole').created().faces()
faces = s3.faces()

Not the same in terms of object identity, but the same in terms of what faces are present. I like both syntaxes, and I think they could both be useful in different situations. One use case for the first way woudl be a case when you want to select faces from > 1 operation, without resorting to tracking both and then unioning them with set arithmetic.

I'm with @fragmuffin that query support like the first line could increase memory usage, and probably should either be very smart, or optionally disableable by the user if they do not need it.

adam-urbanczyk commented 6 years ago

@dcowden

faces1 = ctx.find('hole').created().faces()
faces2 = s3.faces()

I'd argue that those two lines should yield different result (faces1 being a subset of faces2). faces1 only contain faces created in the operation and faces2 all faces of s3. I think this is how the underlying CAD kernel works. For example:

from cadquery import *

box1 = Workplane('XY').box(1,1,1)
box2 = Workplane('XY').box(0.5,0,5,1)
box3 = box1.cut(box2)

will give us the following shape (only box3 is shown)

obraz

If select the '<X' face I will get the following:

>>> box1.faces('<X').toOCC()
class<'TopoDS_Face'; id:887486768>
>>> box3.faces('<X').toOCC()
class<'TopoDS_Face'; id:887486768>

Which to my understanding means that those faces on two different solids are identical for OCC (not necessarily the same objects in memory though).

@fragmuffin I agree with the need to turn such elaborate bookkeeping off. I'd say we need 3 layers with the last being optional:

  1. Shapes (Wires, Shells, Solids) with selectors
  2. Operations that act or create shapes
  3. Operation history with history based selection
dcowden commented 6 years ago

@adam-urbanczyk yes you are right! We are on the same page, that's good. I don't know what i was thinking.

I can implement the layers you describe. My thinking right now is that topology navigation will not be built-into the shapes, but rather the selectors. This will not matter from the viewpoint of either the direct api or the fluent api-- it will simply keep the code more maintainable.

@fragmuffin @adam-urbanczyk I will try to make sure that we have a different layer that handles operation trees and the associated storage, so that we do not incur that overhead unless we need it.

Based on the discussion here, I think i'll also try to keep the complexity of the history separated in the code base. it would be very easy design-wise to have a history and then kind of 'neuter' it when the user doesnt care, but that will make the code less maintainable. I need to go back to the drawing board to figure out a good way to allow the operation history to be added or removed while also keeping the code clear.

As an specific example, operations must communicate somehow which operations they modified and created, if they are capable of doing so. I had previously planned to have BaseOperation do this work using a base class shape history object, with the idea that we would always do it. Now I think i'll try to separate that, so that BaseOperation is simpler and doesnt have anything to do with the shape history. BaseOperation will be cleaner, and we'll have another layer that optionally manages the history.

Thanks to all for the participation in this discussion. We're iterating to a great design.

fragmuffin commented 6 years ago

@adam-urbanczyk @dcowden perhaps a viable compromise would be something like:

# These two return the same:
faces = ctx.find('hole').created().faces()
faces = s3.faces(op_id='hole')
# And this returns all Faces, including those in the `faces` variable
faces2 = s3.faces() 
dcowden commented 6 years ago

I would expect

s3.faces(op_id='hole')

to give all faces of the object that resulted from the cut operation

Perhaps more importantly,I would expect these two to return the same thing

s3.faces() ctx.find(op_id='hole').faces()

In this way, the operation tracking is just a dictionary that the context keeps. Then we don't have to turn the context tracking on or off at a global level at all.... We could only track it if the user specifically provides an operation id, showing intent to query later. If you don't use operation ids, your code is clean and the dictionary remains empty.

fragmuffin commented 6 years ago

That's great! A clean() function could also be made to clear out any history (optional housekeeping) Perhaps clean(pattern='hole*') could remove the results of all operations with an id beginning with hole