Skip to content

Conversation

@jmwright
Copy link
Member

No description provided.

@shimwell
Copy link
Contributor

This PR closes #1954

@codecov
Copy link

codecov bot commented Dec 17, 2025

Codecov Report

❌ Patch coverage is 96.87500% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 95.84%. Comparing base (2e05819) to head (0e5055c).

Files with missing lines Patch % Lines
cadquery/occ_impl/importers/assembly.py 95.23% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1956   +/-   ##
=======================================
  Coverage   95.84%   95.84%           
=======================================
  Files          29       29           
  Lines        7922     7951   +29     
  Branches     1191     1194    +3     
=======================================
+ Hits         7593     7621   +28     
  Misses        192      192           
- Partials      137      138    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jmwright jmwright changed the title Initial materials Support for STEP Export Materials Support for STEP Import/Export Dec 18, 2025
@jmwright
Copy link
Member Author

@adam-urbanczyk When you get a chance, could you take a look at the mypy error on this? I've had similar problems before where every change I make to the AssemblyProtocol overloads seems to break other things. I'm trying not to be too disruptive with this change since I know you put a lot of effort into making this code run correctly, so I do not want to be hacking around a lot trying to figure out how to fix mypy.

mypy error for reference:

cadquery/occ_impl/importers/assembly.py:259: error: No overload variant of "add" of "AssemblyProtocol" matches argument types "AssemblyProtocol", "Location", "str", "Color | None", "Material | None"  [call-overload]
cadquery/occ_impl/importers/assembly.py:259: note: Possible overload variants:
cadquery/occ_impl/importers/assembly.py:259: note:     def add(self, obj: AssemblyProtocol, loc: Location | None = ..., name: str | None = ..., color: Color | None = ...) -> AssemblyProtocol
cadquery/occ_impl/importers/assembly.py:259: note:     def add(self, obj: Shape | Workplane | None, loc: Location | None = ..., name: str | None = ..., color: Color | None = ..., material: Material | str | None = ..., metadata: dict[str, Any] | None = ...) -> AssemblyProtocol

@adam-urbanczyk
Copy link
Member

I think I fixed it, the overloads were not annotated properly.

@jmwright
Copy link
Member Author

@adam-urbanczyk Thanks!

@jmwright jmwright marked this pull request as ready for review December 22, 2025 16:28
Copy link
Member

@adam-urbanczyk adam-urbanczyk left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@jmwright jmwright requested a review from lorenzncode December 29, 2025 20:42
Copy link
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 added a comment, also there is a typo line 1067 test_assembly.py (earlier commit):
# Test the ability to has a material
"to have"

@jmwright
Copy link
Member Author

I'll wait for a day or two before merging in case I missed anything else.

@adam-urbanczyk adam-urbanczyk linked an issue Dec 31, 2025 that may be closed by this pull request
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

Successfully merging this pull request may close these issues.

Add materials to assemblies

5 participants