Support for materials
Implemented material support. In this approach, materials are stored in python dataclasses, the OpenCascade objects are only created when building the assembly. Tests/docs are missing at the moment.
This looks great, I wonder if there is ever a use case for a material to be a string in as an option in addition to these nice material classes you have here. I was wondering if something like this would be possible / useful
assy = cq.Assembly()
assy.add(cube, name="red_cube", material="steel")
@shimwell it would certainly be nice to have some predefined materials, and maybe allow to use the material name for other things too. But let's start with making CI green.
Codecov Report
Attention: Patch coverage is 98.50187% with 4 lines in your changes missing coverage. Please review.
Project coverage is 95.74%. Comparing base (
f754469) to head (4a0bc97). Report is 2 commits behind head on master.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| cadquery/materials.py | 98.80% | 0 Missing and 2 partials :warning: |
| cadquery/occ_impl/assembly.py | 96.61% | 1 Missing and 1 partial :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #1815 +/- ##
==========================================
+ Coverage 95.66% 95.74% +0.07%
==========================================
Files 28 29 +1
Lines 7431 7606 +175
Branches 1122 1150 +28
==========================================
+ Hits 7109 7282 +173
Misses 193 193
- Partials 129 131 +2
: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.
Material support in step exporting will get better in OCCT: https://github.com/Open-Cascade-SAS/OCCT/pull/447
This looks great, I wonder if there is ever a use case for a material to be a string in as an option in addition to these nice material classes you have here. I was wondering if something like this would be possible / useful
assy = cq.Assembly() assy.add(cube, name="red_cube", material="steel")
Would probably nice to have a set of materials to get started, but I think, any real-world application will require custom material sets. Maybe there's another solution and we can find an existing material repository that can be utilized.
(edit: for example https://physicallybased.info)
VRML export runs, but is not correct yet.
The PR would be ready from my side for a first iteration.
I think it would be great to add textures later on, one could create quite good visualizations directly with cadquery/vtk. For the time, the PBR materials already achieve quite nice results.
Thanks for all the hard work! Are the new classes picklable?
Yes, they are pure dataclasses/basic python types, so everything can be pickled.
How far is the progress on the serialization topic? Have you thought about using pydantic? I plan to implement model caching in our internal project and json would be easier to use in a db like with Postgres' JSONB column or redis' JSON datatype compared to pickle.
Perfect, thanks!
There are no plans to implement serialization beyond pickling which is already there. For caching I'd suggest joblib, but maybe you have something else in mind.
@adrianschneider94 @adam-urbanczyk What action items are left to do before getting this PR merged?
Review... Given the size of the PR it might take a while, but we'll definitely get this merged.
I'll plan to spend some time reviewing it next week then.
I see that this branch has conflicts now that I must have missed last week. Should it be rebased?
I see that this branch has conflicts now that I must have missed last week. Should it be rebased?
Rebase is done.
Hey, thanks for all the hard work, but this requires some serious rework with backward compatibility in mind. May I as why there are so many changes that are not needed?
I think there are only two changes that affect backwards compatibility: The missing toTuple() method and the changed signatures of some export methods. Or am I missing something?
The only changes not needed (in my eyes) are the assembly iteration and unifying the color handling. I think both make sense in terms of readability/clarity and as they appear in all places where materials need to be handled, it seemed obvious to me to handle it.
As a suggestion, I will revert the iteration and widen the signatures of all places where colors can be passed, so it's entirely backwards compatible at all those places.
@adrianschneider94 I see the CI is green, congrats on that.
I'm super keen on this feature.
The part I'm most keen on is allowing a simple string for a material. We noticed it is possible to do this using a Material
assy = cq.Assembly()
assy.add(cube, name="red_cube", material=Material("steel"))
I was wondering what are you thoughts on allowing a string as well as Material object, something like this?
assy = cq.Assembly()
assy.add(cube, name="red_cube", material="steel")
We noticed reviewer edits on the branch are disabled so I guess you are the only one who could make these changes if you consider it acceptable.
Anyway it is a nice PR and I would love to see the features suggested in Cq
@shimwell, I'll try to get back to it in the next weeks. Regarding the string materials: What materials would you include, should they be maintained in cadquery? Or would you include a material repository as dependency or fetch from the web? (see also https://github.com/CadQuery/cadquery/pull/1815#issuecomment-2816127608)
Thanks @adrianschneider94
Regarding the string materials: What materials would you include, should they be maintained in cadquery? Or would you include a material repository as dependency or fetch from the web? (see also #1815 (comment))
I would have a local definition of materials. I've not got a fixed list of what string correlates to what color as I don't intend to use then for colors. For me the material strings would be used to look up elemental composition and density of the material for use in physics simulations.
Two questions @adrianschneider94. Could you enable edits by maintainers? Do you intend to address the review comments?
I think it would be better from a compatibility point of view, to have material as a key in the metadata dict for each component of the assembly. This way the call signatures to iterators are not changed by the addition of materials.
Metadata are meant for custom metadata (which will be eventually supported when exporting to STEP or CAF). Anyways, we will handle this PR after the release. Most likely it will be significantly reworked and split up.
The option to allow edits does not appear, I think because the repo is owned by an organization: https://github.com/orgs/community/discussions/5634
Yes, I will address the comments, when I have time again to work on this.