Part.makeRevolution() and Shape revolve() - bugs or misunderstandings

[PaulG]

(I've adjusted the subject line to reflect more recent content in the thread)

Following seems pretty clearly to be a bug, but I am seeking comment on the actual problem before putting it in the tracking system. (Using 0.19 #16694 (Git), details below.)

Under certain circumstances Part.makeRevolution() fails indicating "TypeError: function takes at most 6 arguments (7 given)"

The documentation indicates 7 arguments:

Code: Select all

Part.makeRevolution.__doc__
'makeRevolution(Curve,[vmin,vmax,angle,pnt,dir,shapetype]) -- Make a revolved shape\nby rotating the curve or a portion of it around an axis given by (pnt,dir).\nBy default vmin/vmax=bounds of the curve,angle=360,pnt=Vector(0,0,0) and\ndir=Vector(0,0,1) and shapetype=Part.Solid'

This is from line 353 in FreeCAD/src/Mod/Part/App/AppPartPy.cpp. The code works properly for

Code: Select all

import Part
from FreeCAD import Vector

z = [ Vector(0, 0, 0) , Vector(0, .25, .5),  Vector(0, .5, 1.0), Vector(0, .25, 1.5)]
c=Part.BSplineCurve()
c.interpolate(z)       

cS = Part.makeRevolution(c, c.FirstParameter, c.LastParameter, 360, 
        Vector(0,0,0), Vector(0,0,1), Part.Solid) 

which has 7 arguments, but the following with 7 arguments

Code: Select all

eC = c.toShape()
cS = Part.makeRevolution(eC, eC.FirstParameter, eC.LastParameter, 360, 
        Vector(0,0,0), Vector(0,0,1), Part.Solid)  

fails giving TypeError: function takes at most 6 arguments (7 given)

At the very least the code and error message do not agree. But looking (with no understanding of cpp) at the code following line1307 in FreeCAD/src/Mod/Part/App/AppPartPy.cpp it seems that it may be able to extract the curve from the shape object.

So I think the problem may be that the argument count check is throwing an incorrect error and preventing the code from working. Could someone with some understanding of cpp please comment on this.

OS: Linux Mint 18.1 Serena (X-Cinnamon/cinnamon)
Word size of OS: 64-bit
Word size of FreeCAD: 64-bit
Version: 0.19.16694 (Git)
Build type: Unknown
Branch: master
Hash: cd74f2e92908e94f9f7bc718e1be52939647b844
Python version: 2.7.12
Qt version: 4.8.7
Coin version: 4.0.0a
OCC version: 7.3.0
Locale: English/UnitedStates (en_US)

[DeepSOIC]

The error message complaining about 6 arguments at most is indeed a bug. It comes from doing PyArg_ParseTuple(args.ptr(), "O!|dddO!O!"... last, and is the error message generated by ParseTuple routine. We do two parseTuple in a row, but only the error message from the last one is ever shown.

From C++ implementation it is clear to me, that if you give Part.Geometry-based object (BSplineCurve is one) as the first argument, all 7 arguments are supported, including the last one (ShapeType). But if the first argument is Part.Shape-based object, the last argument is not supported - the type of generated shape is fully determined by the type of shape passed in.
So, the docstring should be corrected as to when the last argument is supported, and the error message should be improved.

[PaulG]

Ok, thanks. Should I file a bug report or will you look after it?

[PaulG]

I'm also curious why I get

Code: Select all

>>> cS.isValid()
False

[DeepSOIC]

Ok, thanks. Should I file a bug report or will you look after it?

Please do make a bug report. I'm working on another thing, and don't want to mix in other random changes. Then I will for sure just forget about it.

[DeepSOIC]

I'm also curious why I get

Code: Select all

>>> cS.isValid()
False

I just revolved your bspline in gui. It creates an open shell. The shell must be closed to be able to make a solid.

I'm not sure about how this function handles the request to create a solid. In Gui, to make a solid with a revolution, one must revolve a closed wire. But in GUI, one always operates on Shape objects, not Part.Geometry. So here it's probably something different.

[DeepSOIC]

makeRevolution appears to be invoking OCC method directly more-or-less.
The docs are scarce.
https://www.opencascade.com/doc/occt-7. ... dalg_4_1_8
https://www.opencascade.com/doc/occt-7. ... ution.html

[wmayer]

When looking at the git history the additional parameter shapetype was introduced in git commit 80bbd3f2af0. Now when restudying the code I think it was an oversight not to support the shapetype parameter in case the first is a shape.

The only difference of what is done if a curve or an edge is passed is that for the latter case an internal curve is created with the placement of the edge applied to it. But in the end the same algorithm is used to compute the revolution.

The shapetype parameter is only used to force the output shape type.

[wmayer]

git commit f6ee90557

[PaulG]

I just revolved your bspline in gui. It creates an open shell. The shell must be closed to be able to make a solid.

Sorry about the bad (not closed) example with Bspline. I should have looked at it more carefully.