Add STEP Import for Assemblies

[jmwright]

The goal is to make it possible to round-trip assemblies to and from STEP without loss of data. This data can include:

• [x] Part location
• [x] Part color
• [x] Shape colors
• [x] Shape colors
• [x] Shape names (advanced face)

[codecov[bot]]

Codecov Report

:x: Patch coverage is 96.56160% with 12 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 96.06%. Comparing base (8431073) to head (353c286). :warning: Report is 5 commits behind head on master.

Files with missing lines	Patch %	Lines
cadquery/occ_impl/importers/assembly.py	96.29%	1 Missing and 5 partials :warning:
cadquery/occ_impl/assembly.py	93.93%	0 Missing and 4 partials :warning:
cadquery/assembly.py	97.59%	0 Missing and 2 partials :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1779      +/-   ##
==========================================
+ Coverage   95.66%   96.06%   +0.39%     
==========================================
  Files          28       30       +2     
  Lines        7431     9444    +2013     
  Branches     1122     1562     +440     
==========================================
+ Hits         7109     9072    +1963     
- Misses        193      226      +33     
- Partials      129      146      +17     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jmwright]

@adam-urbanczyk In 4045c5813708dbbd497f2504c5ece94927b8a689 you set the cadquery.Assembly.importStep method up to be a class method, and you construct assy before calling the lower-level method. However, since the Assembly.name property is private and can only be set during instantiation, this creates an issue for me. I need to be able to set the top-level assembly name based on the name set in the STEP file, which requires me to set the name property after instantiation.

We need to decide on the proper way to fix this.

[adam-urbanczyk]

Hey, I do not understand the issue. You can always do this:

assy = Assembly()
assy.name = '123'

Do you mean that you need to modify AssemblyProtocol to satisify mypy? That also does not sound like an issue to me.

[jmwright]

Correct, mypy complains. I can make that change to make name a public property if you don't see an issue with doing so.

[jmwright]

@adam-urbanczyk @lorenzncode I think this is ready for a look when you get a chance.

[lorenzncode]

I tested this branch with the "door" example from the docs. I made only a color modification to set vslot color to white.

The step file as displayed in FreeCAD:

After importStep displayed with cadquery.vis:

It looks like locations are not preserved in this case?

[jmwright]

Thanks @lorenzncode I thought I had that fixed. I'll figure out what's going on.

[adam-urbanczyk]

I can confirm the issue. It seems like locations are ignored. Additionally, import/export roundtrip seems to distort colors.

[jmwright]

Thanks. The colors are probably being shifted because I did not merge master into this branch yet to catch the recent color changes. The location will take some more digging. I have a check or two in the tests to make sure that the location is preserved during import, and those pass. I'll use the the door as an example to make sure it gets properly fixed.

[jmwright]

@adam-urbanczyk Actually, if I just run the door assembly code from the docs and display it with cadquery.vis.show, some colors are shifted/wrong. The STEP import/export feature was NOT used for this screenshot.

[jmwright]

@adam-urbanczyk The gold color of the v-slot parts above is because there is no color defined for those components, and cadquery.vis.show is defaulting to a different color than CQ-editor and the 3D viewer in the documentation. So I think it's a non-issue unless we want to standardize on a default color across the visualization platforms.

@lorenzncode @adam-urbanczyk Can you provide the steps to reproduce the location issue with the constrainted door assembly? I cannot reproduce, so we are following different processes somehow. The code below has all the components in the correct location when I show it using cadquery.vis.show.

import cadquery as cq
from cadquery.occ_impl.exporters.assembly import exportStepMeta
from cadquery.vis import show

# Parameters
H = 400
W = 200
D = 350

PROFILE = cq.importers.importDXF("vslot-2020_1.dxf").wires()

SLOT_D = 6
PANEL_T = 3

HANDLE_D = 20
HANDLE_L = 50
HANDLE_W = 4


def make_vslot(l):
    return PROFILE.toPending().extrude(l)


def make_connector():
    rv = (
        cq.Workplane()
        .box(20, 20, 20)
        .faces("<X")
        .workplane()
        .cboreHole(6, 15, 18)
        .faces("<Z")
        .workplane(centerOption="CenterOfMass")
        .cboreHole(6, 15, 18)
    )

    # tag mating faces
    rv.faces(">X").tag("X").end()
    rv.faces(">Z").tag("Z").end()

    return rv


def make_panel(w, h, t, cutout):
    rv = (
        cq.Workplane("XZ")
        .rect(w, h)
        .extrude(t)
        .faces(">Y")
        .vertices()
        .rect(2 * cutout, 2 * cutout)
        .cutThruAll()
        .faces("<Y")
        .workplane()
        .pushPoints([(-w / 3, HANDLE_L / 2), (-w / 3, -HANDLE_L / 2)])
        .hole(3)
    )

    # tag mating edges
    rv.faces(">Y").edges("%CIRCLE").edges(">Z").tag("hole1")
    rv.faces(">Y").edges("%CIRCLE").edges("<Z").tag("hole2")

    return rv


def make_handle(w, h, r):
    pts = ((0, 0), (w, 0), (w, h), (0, h))

    path = cq.Workplane().polyline(pts)

    rv = (
        cq.Workplane("YZ")
        .rect(r, r)
        .sweep(path, transition="round")
        .tag("solid")
        .faces("<X")
        .workplane()
        .faces("<X", tag="solid")
        .hole(r / 1.5)
    )

    # tag mating faces
    rv.faces("<X").faces(">Y").tag("mate1")
    rv.faces("<X").faces("<Y").tag("mate2")

    return rv


# define the elements
door = (
    cq.Assembly()
    .add(make_vslot(H), name="left")
    .add(make_vslot(H), name="right")
    .add(make_vslot(W), name="top")
    .add(make_vslot(W), name="bottom")
    .add(make_connector(), name="con_tl", color=cq.Color("black"))
    .add(make_connector(), name="con_tr", color=cq.Color("black"))
    .add(make_connector(), name="con_bl", color=cq.Color("black"))
    .add(make_connector(), name="con_br", color=cq.Color("black"))
    .add(
        make_panel(W + 2 * SLOT_D, H + 2 * SLOT_D, PANEL_T, SLOT_D),
        name="panel",
        color=cq.Color(0, 0, 1, 0.2),
    )
    .add(
        make_handle(HANDLE_D, HANDLE_L, HANDLE_W),
        name="handle",
        color=cq.Color("yellow"),
    )
)

# define the constraints
(
    door
    # left profile
    .constrain("left@faces@<Z", "con_bl?Z", "Plane")
    .constrain("left@faces@<X", "con_bl?X", "Axis")
    .constrain("left@faces@>Z", "con_tl?Z", "Plane")
    .constrain("left@faces@<X", "con_tl?X", "Axis")
    # top
    .constrain("top@faces@<Z", "con_tl?X", "Plane")
    .constrain("top@faces@<Y", "con_tl@faces@>Y", "Axis")
    # bottom
    .constrain("bottom@faces@<Y", "con_bl@faces@>Y", "Axis")
    .constrain("bottom@faces@>Z", "con_bl?X", "Plane")
    # right connectors
    .constrain("top@faces@>Z", "con_tr@faces@>X", "Plane")
    .constrain("bottom@faces@<Z", "con_br@faces@>X", "Plane")
    .constrain("left@faces@>Z", "con_tr?Z", "Axis")
    .constrain("left@faces@<Z", "con_br?Z", "Axis")
    # right profile
    .constrain("right@faces@>Z", "con_tr@faces@>Z", "Plane")
    .constrain("right@faces@<X", "left@faces@<X", "Axis")
    # panel
    .constrain("left@faces@>X[-4]", "panel@faces@<X", "Plane")
    .constrain("left@faces@>Z", "panel@faces@>Z", "Axis")
    # handle
    .constrain("panel?hole1", "handle?mate1", "Plane")
    .constrain("panel?hole2", "handle?mate2", "Point")
)

# solve
door.solve()

success = exportStepMeta(door, "door.step")

if success:
    imported_assy = cq.Assembly.importStep("door.step")
    show(imported_assy)

[lorenzncode]

Can you provide the steps to reproduce the location issue

Try export instead of exportStepMeta.

door.export("door.step")

STEP is exported in the assembly tests. I'm getting a ValueError on importStep with one of them.

run the pytest, export step (still uses old save method)

pytest tests/test_assembly.py --basetemp=./tmp -k test_colors_assy1

This fails with ValueError:

from cadquery import Assembly

obj = Assembly.importStep("./tmp/assembly10/chassis0_assy.step")

[adam-urbanczyk]

Here is mine:

before: after:

I also found another issues, without copies the import throws. See last section of the snippet.

#%% imports
from cadquery import Assembly, Location, Color
from cadquery.func import box, rect
from cadquery.vis import show

#%% prepare the model
def make_model(name: str, COPY: bool):
    b = box(1,1,1)
    
    assy = Assembly(name='test_assy')
    assy.add(box(1,2,5), color=Color('green'))
    
    
    
    for v in rect(10,10).vertices():
        assy.add(
            b.copy() if COPY else b,
            loc=Location(v.Center()),
            color=Color('red')
        )
        
    # show(assy)
    
    assy.export(name)
    
    return assy
    
assy_copy = make_model("test_assy_copy.step", True)
assy = make_model("test_assy.step", False)

show(assy)

#%% import the assy with copies

assy_i = Assembly.importStep("test_assy_copy.step")
show(assy_i)

#%% import the assy without copies - this throws

assy_i = Assembly.importStep("test_assy.step")
show(assy_i)

[jmwright]

@adam-urbanczyk @lorenzncode The parent location was not being applied correctly when the original assembly.export method is used. I've fixed that now and pulled in the srgb changes from master, so I think the location and color issues should be fixed.

[adam-urbanczyk]

Do you plan to fix the second issue I found?

[jmwright]

@adam-urbanczyk Yes, I'll see what I can figure out.

[jmwright]

@adam-urbanczyk The Assembly.export method is actually duplicating the names throughout the STEP file, so it does not look like it is a problem with this importer. I find 14 different entries with that UUID in the STEP file. I can add a check to protect against duplicate names being read from the STEP file which alters the duplicate names, but I am not sure we should mask the root cause (duplicated names within the STEP file). It seems like the existing behavior of lettingAssembly.add throw the error could be the right thing to do here.

In the screenshot: Two variations of the name, one with the _part postfix and one without. Those are the only two variations of the name throughout the STEP file.

[adam-urbanczyk]

This tool (eDrawings) can import the problematic STEP without copies and the structure LGTM. I think that CQ should be able to handle this as well (and produce identical structure).

[jmwright]

How do we do that without breaking things like constraints that use part names? We would have to remove the Assembly.add restriction on duplicate names, correct?

[adam-urbanczyk]

But subassy names are unique and parts don't have names in cq AFAIR.

[adam-urbanczyk]

Additional observation:

In [3]: assy_i.objects
Out[3]:
{'a9a08701-4ac0-11f0-a634-c86e08a4b6ef': <cadquery.assembly.Assembly at 0x1b1cf1dcbc0>,
 'a7644761-4ac0-11f0-bcb0-c86e08a4b6ef_part': <cadquery.assembly.Assembly at 0x1b1d123ab10>,
 'a76594bb-4ac0-11f0-9013-c86e08a4b6ef_part': <cadquery.assembly.Assembly at 0x1b1d1238440>,
 'a7668644-4ac0-11f0-91c5-c86e08a4b6ef_part': <cadquery.assembly.Assembly at 0x1b1d123ac30>,
 'a7668645-4ac0-11f0-9a6c-c86e08a4b6ef_part': <cadquery.assembly.Assembly at 0x1b1d123ad20>,
 'a7668646-4ac0-11f0-abef-c86e08a4b6ef_part': <cadquery.assembly.Assembly at 0x1b1d123b680>}

In [4]: assy.objects
Out[4]:
{'test_assy': <cadquery.assembly.Assembly at 0x1b1d11fcc20>,
 'a77ae412-4ac0-11f0-804d-c86e08a4b6ef': <cadquery.assembly.Assembly at 0x1b1d123a090>,
 'a77ae413-4ac0-11f0-ab48-c86e08a4b6ef': <cadquery.assembly.Assembly at 0x1b1d123a480>,
 'a77ae414-4ac0-11f0-b508-c86e08a4b6ef': <cadquery.assembly.Assembly at 0x1b1d123a510>,
 'a77ae415-4ac0-11f0-b410-c86e08a4b6ef': <cadquery.assembly.Assembly at 0x1b1d123a5d0>,
 'a77ae416-4ac0-11f0-b4a6-c86e08a4b6ef': <cadquery.assembly.Assembly at 0x1b1d123a690>}

It seems that the name is taken from the wrong level of the tree.

[jmwright]

It turns out that the exportStepMeta() method has the same issue of applying names to the wrong level. It tries to flatten the assembly by default, and the name of a child-assembly gets applied to the shape(s) within the child-assembly and not to the assembly itself.

I need to create another PR to fix nesting and name handling in exportStepMeta(), and will probably make an option to flatten the assembly (flattening has been asked for by users), but it won't be the default.

I think there are still issues lurking with having duplicate names, but I will fix the name handling in this PR and make sure the nesting works properly. Then I can worry about the unique naming issues.

[adam-urbanczyk]

I see that you are working on #1858, but wouldn't it be better to fix the issue here iso adding (before the release) more functionality? I'd propose to add a heuristic that detect nodes with one child and _part suffix and take the shape from _part and name from the parent. IMO this does not sound too complicated.

[jmwright]

#1858 was created because there is an oversight within #1782 that causes exportStepMeta to act differently than the standard Assembly.export method. I'm trying to fix that, not adding functionality, other than adding a flatten parameter because I expect it to be asked for eventually.

[jmwright]

The structure looks the same now on the test case that used to fail. The first screenshot is the one that was exported, then the second is that STEP imported, then re-exported.

[adam-urbanczyk]

OK, I'll take one final look.

[lorenzncode]

I am still getting a ValueError with a STEP from the tests.

(cqdev) lorenzn@fedora:~/devel/cadquery$ git status -u no
On branch assembly-import
Your branch is up to date with 'upstream/assembly-import'.

nothing to commit, working tree clean
(cqdev) lorenzn@fedora:~/devel/cadquery$ pytest tests/test_assembly.py --basetemp=./tmp2 -k test_colors_assy1 -q --no-summary
...........                                                                                                                                                        [100%]
11 passed, 96 deselected, 13 warnings in 0.58s
sys:1: DeprecationWarning: builtin type swigvarlink has no __module__ attribute
(cqdev) lorenzn@fedora:~/devel/cadquery$ cat test2.py 
from cadquery import Assembly

obj = Assembly.importStep("./tmp2/assembly10/chassis0_assy.step")
(cqdev) lorenzn@fedora:~/devel/cadquery$ python test2.py 
Traceback (most recent call last):
  File "/home/lorenzn/devel/cadquery/test2.py", line 3, in <module>
    obj = Assembly.importStep("./tmp2/assembly10/chassis0_assy.step")
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/lorenzn/devel/cadquery/cadquery/assembly.py", line 622, in importStep
    _importStep(assy, path)
  File "/home/lorenzn/devel/cadquery/cadquery/occ_impl/importers/assembly.py", line 194, in importStep
    process_label(labels.Value(1))
  File "/home/lorenzn/devel/cadquery/cadquery/occ_impl/importers/assembly.py", line 173, in process_label
    process_label(sub_label, loc, name)
  File "/home/lorenzn/devel/cadquery/cadquery/occ_impl/importers/assembly.py", line 149, in process_label
    process_label(ref_label, parent_location, parent_name)
  File "/home/lorenzn/devel/cadquery/cadquery/occ_impl/importers/assembly.py", line 173, in process_label
    process_label(sub_label, loc, name)
  File "/home/lorenzn/devel/cadquery/cadquery/occ_impl/importers/assembly.py", line 149, in process_label
    process_label(ref_label, parent_location, parent_name)
  File "/home/lorenzn/devel/cadquery/cadquery/occ_impl/importers/assembly.py", line 173, in process_label
    process_label(sub_label, loc, name)
  File "/home/lorenzn/devel/cadquery/cadquery/occ_impl/importers/assembly.py", line 149, in process_label
    process_label(ref_label, parent_location, parent_name)
  File "/home/lorenzn/devel/cadquery/cadquery/occ_impl/importers/assembly.py", line 173, in process_label
    process_label(sub_label, loc, name)
  File "/home/lorenzn/devel/cadquery/cadquery/occ_impl/importers/assembly.py", line 149, in process_label
    process_label(ref_label, parent_location, parent_name)
  File "/home/lorenzn/devel/cadquery/cadquery/occ_impl/importers/assembly.py", line 179, in process_label
    _process_simple_shape(label, parent_location, parent_name)
  File "/home/lorenzn/devel/cadquery/cadquery/occ_impl/importers/assembly.py", line 74, in _process_simple_shape
    assy.add(
  File "/home/lorenzn/devel/cadquery/cadquery/assembly.py", line 241, in add
    self.add(assy)
  File "/home/lorenzn/devel/cadquery/cadquery/assembly.py", line 222, in add
    raise ValueError("Unique name is required")
ValueError: Unique name is required

[adam-urbanczyk]

@jmwright any updates on this PR?

[jmwright]

@adam-urbanczyk Not yet. I got busy with some other things.

[jmwright]

@adam-urbanczyk I'm getting mypy errors now that I did not before. Most of them are probably my fault, but there is this one about the OpenCASCADE code.

cadquery\occ_impl\importers\assembly.py:105: error: "TDF_Attribute" has no attribute "GetFather"  [attr-defined]

Does this have anything to do with the stubs being published as part of OCP 7.9.x?