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

Make Workplane Work Better as a Base Class #673

Closed
pavpen opened this issue Mar 3, 2021 · 5 comments
Closed

Make Workplane Work Better as a Base Class #673

pavpen opened this issue Mar 3, 2021 · 5 comments

Comments

@pavpen
Copy link
Contributor

pavpen commented Mar 3, 2021

As a CadQuery user, and developer of custom plugins, I would like to be able to derive classes from Workplane that add functionality, such as methods for generating curves I may often use. Currently, deriving a class from Workplane produces class, which can't directly use the inherited fluent API methods.

Notes

Why Inheritance Breaks

Workplane.newObject() is called by many of the methods that add points, curves, wires, or solids to the workplane stack. The newObject() method instantiates the result returned by the fluent API methods like this:

        # copy the current state to the new object
        ns = Workplane()
        # . . .
        return ns

The result is that any instance of a class derived from cq.Workplane would inherit fluent API methods that return instances of cq.Workplane, rather than instances of the derived class.

Example

E.g.:

>>> import cadquery
>>> class ExtendedWorkplane(cadquery.Workplane):
...     def newShape(self):
...             print('Fancy.')
... 
>>> result = ExtendedWorkplane('XY')

We want to still have an ExtendedWorkplane after result.moveTo(), but:

>>> type(result.moveTo(1, 2))
<class 'cadquery.cq.Workplane'>
>>> type(result)
<class '__main__.ExtendedWorkplane'>
>>> result.newShape()
Fancy.
>>> result.moveTo(1, 2).newShape()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'Workplane' object has no attribute 'newShape'

What a Fix would Look Like

The following implementation of newObject() will fix this problem:

        # copy the current state to the new object
        ns = type(self)()
        # . . .
        return ns

A change to the return types of the Workplane methods calling newObject() would also be needed to satisfy type checkers.

Opinion

In my view, class inheritance is a better way to add plugins to CadQuery's Workplane, rather than Workplane.someMethodName = functionExpression. Inheritance works better with code autocompletion and analysis, such as type checking, and provides a way to name plugins, and put them in a hierarchy, in order to avoid possible name collisions if multiple users try to add a method with the same name to Workplane.

Let me know what you think, I'd be willing to implement this when I get a bit of time.

@adam-urbanczyk
Copy link
Member

If you need it, then no problem. Just use self.__class__() (same style was used elsewhere in the codebase AFAIR). I don't understand the point with the return type - what is the error you are getting with mypy?

Keep in mind that if you want to share your plugins they might be not easily usable by others if implemented as subclasses.

@pavpen
Copy link
Contributor Author

pavpen commented Mar 4, 2021

Keep in mind that if you want to share your plugins they might be not easily usable by others if implemented as subclasses.

Is there a repository of contributed plugins?

I was thinking more of having the ability to ensure I'm using a workplane with the plugins I expect to exist. E.g., something like:

class WorkplaneWithSpirals(cadquery.Workplane):
    def archimedeanSpiral(parameters):
        # . . . Implementation . . .

result = WorkplaneWithSpirals("XY").archimedeanSpiral(parameters)

In some other module:

class WorkplaneWithMetricBoltProfile(WorkplaneWithSpirals):
    def metricBoltProfile(parameres):
        return self.archimedeanSpiral(parameters1).archimedeanSpiral(parameters2) # . . .

result = WorkplaneWithMetricBoltProfile("XY").metricBoltProfile(parameters).twistExtrude(
        distance = pitch * thread_turns, angle = 2 * math.pi * thread_turns)

I don't understand the point with the return type - what is the error you are getting with mypy?

Interestingly, in the example above, mypy doesn't complain about calling a method on an instance of a class that doesn't define that method. The Visual Studio Code code completion, however, does seem to depend on the proper return type definition.

The code in the screen capture below is similar to the example ExtendedWorkplane above. However, it has a typedMoveTo() method which has a return type that's the same as whatever the type of the self argument is. The moveTo() method from the parent cadquery.Workplane class is inherited with its type hint (declaring moveTo as returning type cadquery.Workplane).

from typing import Iterable, TypeVar, cast

import cadquery

SelfT = TypeVar("SelfT")


class ExtendedWorkplane(cadquery.Workplane):
    # . . .

    def typedMoveTo(self: SelfT, x: float, y: float) -> SelfT:
        # Keeping `mypy` happy here requires a few casts:
        return cast(
            SelfT,
            super(ExtendedWorkplane, cast(ExtendedWorkplane, self)).moveTo(x=x, y=y),
        )

    def newShape(self):
        print("Fancy")

        return self

Of course, cadquery.Workplane doesn't have a newShape() method. So, trying to call newShape() on the result of moveTo() (which is declared as having type cadquery.Workplane) should produce a type hint error.

ExtendedWorkplane does have newShape(). And when you call typedMoveTo() on an object of type ExtendedWorkplane, the type hint declares the result as having the type of self, which is ExtendedWorkplane in this case. So, the result of ExtendedWorkplane(...).typedMoveTo(...) is of type ExtendedWorkplane and does have a newShape() method.

So, you can see code completion suggests newShape after calling typedMoveTo(), but not after calling MoveTo().

cadquery-1614845092062

@jmwright
Copy link
Member

jmwright commented Mar 4, 2021

Is there a repository of contributed plugins?

We could use cadquery-contrib, but I would probably rather create a cadquery-plugins repo. @pavpen If you want to contribute plugins, I can create that repo and post the link here.

@pavpen
Copy link
Contributor Author

pavpen commented Mar 5, 2021

It makes no difference to me. I could contribute to this repository or another one, whatever is easier for you to manage. I'll need to spend some time to make sure my code is good enough for other people to see. (E.g., calculating the deviation of a B-spline from an Archimedean spiral, so you can stay within a given tolerance.)

Also are there some criteria for what should go into the core Workplane, and what should be separate? Perhaps, the core Workplane should be lightweight, and other common or less common curves and solids should be in separate classes.

@pavpen
Copy link
Contributor Author

pavpen commented Mar 12, 2021

Resolved by #677.

@pavpen pavpen closed this as completed Mar 12, 2021
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

No branches or pull requests

3 participants