Improved error when pendingWires/pendingEdges is empty

[marcus7070]

Minimal example:

w = (
    cq.Workplane()
    .box(10, 10, 10)
    .faces(">Z")
    .edges()
    .extrude(3, taper=40)
)

Result is IndexError: list index is out of range in CQ-editor.

A more detailed trace from the interpreter is still pretty opaque:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/nix/store/sgx51ii0rf6872aj5ab5qvf5fw3kdll4-python3-3.8.6-env/lib/python3.8/site-packages/cadquery/cq.py", line 2745, in extrude
    r = self._extrude(
  File "/nix/store/sgx51ii0rf6872aj5ab5qvf5fw3kdll4-python3-3.8.6-env/lib/python3.8/site-packages/cadquery/cq.py", line 3177, in _extrude
    thisObj = Solid.extrudeLinear(ws[0], [], eDir, taper)
IndexError: list index out of range

To be clear, the error is caused by ctx.pendingWires and pendingEdges being empty. I need to push the results from .edges() to them by calling toPending() before extruding.

Here are some issues where other users have run into this, and I'm sure I'm missing a few more:

• cutting a groove #619 (comment)
• How to extrude a polyline without mirror? #546 (comment) (although this one has Edges in pendingEdges, just an empty pendingWires causing the error)

So question 1, we could use a more informative error than IndexError for when we expect a pendingWire or Edge but get nothing, right?

I think these methods would all fail with an IndexError if used like above:

• twistExtrude
• cutThruAll
• loft
• _extrude
• _revolve
• _sweep

You could check in each Workplane method if ctx.pendingWires was empty, but to save code repetition I suggest we add a new method to CQContext that either will either return a pending wire/edge or raise an error like RuntimeError('Requested pending wires but none present in modelling context'). Then change the relevant methods to use this getter instead of directly accessing ctx.pendingWires. Also this would need a pop=True keyword option, considering that almost every method does:

        wires = self.ctx.pendingWires
        self.ctx.pendingWires = []

Question 2: Does this sound like a good approach?

[jmwright]

@marcus7070

So question 1, we could use a more informative error than IndexError for when we expect a pendingWire or Edge but get nothing, right?

I think long-term for usability to an ever-broadening range of users, better error messages would be a good idea. There are lots of places where higher level error messages would be useful. Keeping everything clean while doing that will be important though.

Question 2: Does this sound like a good approach?

I think so. Having a getter that raises the exception is probably a good way to do it. Are you thinking of adding a matching setter too, or will pendingWires still be set directly?

[adam-urbanczyk]

@marcus7070 I think it would be nice to implement pendingWires as a property of CQContext.

[marcus7070]

My plan was to keep ctx.pendingWires as a regular attribute (and keep adding pending wires directly), and add something like:

class CQContext:
    ...
    def getPending(edges=False, wires=True):
        rv = []
        if edges:
            rv.extend(self.pendingEdges)
            self.pendingEdges = []
        if wires:
            rv.extend(self.pendingWires)
            self.pendingWires = []
        if not rv:
            raise RuntimeError('Pending edges/wires requested but none present')
        return rv

Then this:

cadquery/cadquery/cq.py

Lines 3083 to 3084 in 184a985

	wires = self.ctx.pendingWires
	self.ctx.pendingWires = []

would become

    wires = ctx.getPending()

I think it would be nice to implement pendingWires as a property of CQContext.

I'm not so sure about using a full property decorator on this, since a user inspecting pending wires with something like print(w.ctx.pendingWires) could generate a RuntimeError. Sure, we could just instruct them to use ctx._pendingWires, but I think having a getPending method is more clear.

Unless you have further plans with pending wires that would need an @property implementation?

[jmwright]

Unless you have further plans with pending wires that would need an @Property implementation?

I don't have any further plans.