Skip to content

Added quality controls to STEP export for shape and assembly#1083

Merged
jmwright merged 4 commits into
masterfrom
step
May 27, 2022
Merged

Added quality controls to STEP export for shape and assembly#1083
jmwright merged 4 commits into
masterfrom
step

Conversation

@jmwright
Copy link
Copy Markdown
Member

@jmwright jmwright commented May 13, 2022

This is in support of the KiCAD model generator, per #1082

What is accepted via kwargs can be added to over time, but for now there are just 2 parameters that influence file size.

@jmwright jmwright marked this pull request as draft May 13, 2022 20:41
@jmwright jmwright mentioned this pull request May 13, 2022
Closed
@codecov
Copy link
Copy Markdown

codecov Bot commented May 13, 2022 •
edited
Loading

Codecov Report

Merging #1083 (41cc18a) into master (cee66ff) will decrease coverage by 0.00%.
The diff coverage is 95.12%.

❗ Current head 41cc18a differs from pull request most recent head bda8995. Consider uploading reports for the commit bda8995 to get more accurate results

@@            Coverage Diff             @@
##           master    #1083      +/-   ##
==========================================
- Coverage   96.37%   96.36%   -0.01%     
==========================================
  Files          40       40              
  Lines        9496     9529      +33     
  Branches     1256     1259       +3     
==========================================
+ Hits         9152     9183      +31     
- Misses        202      203       +1     
- Partials      142      143       +1
Impacted Files Coverage Δ
cadquery/occ_impl/exporters/__init__.py 91.58% <33.33%> (-1.75%) ⬇️
cadquery/assembly.py 96.55% <100.00%> (ø)
cadquery/occ_impl/exporters/assembly.py 100.00% <100.00%> (ø)
cadquery/occ_impl/shapes.py 92.60% <100.00%> (+0.04%) ⬆️
tests/test_assembly.py 100.00% <100.00%> (ø)
tests/test_exporters.py 99.19% <100.00%> (+0.05%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@jmwright
Copy link
Copy Markdown
Member Author

jmwright commented May 18, 2022

@lorenzncode I'm adding tests for the single object and assembly STEP exports, and setting them up to use the temporary directory method you've been using. test_assembly.test_step_export worked fine when I added tmp_path_factory as an argument, but when I tried the same thing with test_exporters.testSTEPOptions I got this error:

TypeError: TestExporters.testSTEPOptions() missing 1 required positional argument: 'tmp_path_factory'

Am I missing a decorator or something?

@lorenzncode
Copy link
Copy Markdown
Member

lorenzncode commented May 18, 2022

@jmwright I'd suggest to move the new tests outside of the TestExporters class to define as a pytest test rather than unittest. TestExporters is a subclass of unittest.TestCase.

@jmwright jmwright marked this pull request as ready for review May 18, 2022 14:22
@jmwright jmwright changed the title [WIP] Added quality controls to STEP export for shape and assembly Added quality controls to STEP export for shape and assembly May 18, 2022
@jmwright
Copy link
Copy Markdown
Member Author

jmwright commented May 18, 2022

Ok @adam-urbanczyk @lorenzncode I think this is ready for review.

@lorenzncode
STEP export
bda8995 
* exporters.export: expose STEP options
* Use default OCCT write.precision.mode value 0
* Doc update

Merge branch 'master' into step
lorenzncode
lorenzncode approved these changes May 21, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@lorenzncode lorenzncode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested this branch and confirm file size decrease with write_pcurves False. I also confirm the uncertainty value change with change in write.precision.mode.

I made some minor changes:

  • exporters.export: expose STEP options
  • Use default OCCT write.precision.mode value 0
  • Doc update

Regarding docstrings, my preference for these export options, would be to defer to OCCT documentation for the most part. I've modified the docstring to provide short descriptions only, rather than try to reinterpret the meanings especially for write.precision.mode.

@jmwright
Copy link
Copy Markdown
Member Author

jmwright commented May 21, 2022

@lorenzncode Thanks, these changes look good to me.

@jmwright
Copy link
Copy Markdown
Member Author

jmwright commented May 26, 2022

@adam-urbanczyk Any objections to me merging this?

@jmwright jmwright merged commit 3032e0f into master May 27, 2022
@jmwright jmwright deleted the step branch May 27, 2022 10:16
@mojca mojca mentioned this pull request Aug 29, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adam-urbanczyk adam-urbanczyk adam-urbanczyk approved these changes

@lorenzncode lorenzncode lorenzncode approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jmwright @lorenzncode @mojca @adam-urbanczyk