-
Notifications
You must be signed in to change notification settings - Fork 396
Add Basic Materials Functionality #1923
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1923 +/- ##
==========================================
+ Coverage 95.81% 95.84% +0.02%
==========================================
Files 29 29
Lines 7867 7922 +55
Branches 1186 1191 +5
==========================================
+ Hits 7538 7593 +55
Misses 192 192
Partials 137 137 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
adam-urbanczyk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand the intent of this PR correctly, you actually want to use XCAFDoc_Material
|
@adam-urbanczyk I need to better understand what you are thinking. The current |
|
In the end this class should wrap NB: it can be used for now to simply store a name. |
|
@adam-urbanczyk How about this? |
|
Ok, let me take a look. |
adam-urbanczyk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good in general, but could you implement pickling? If it is too much work you could for now get rid of the vis part.
@adam-urbanczyk I'll see what I can do. |
|
FYI: here is how it is done for cadquery/cadquery/occ_impl/assembly.py Line 145 in fea85a5
|
|
@adam-urbanczyk I tried to bring this PR up to par with the Color class. |
adam-urbanczyk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but please check if you need all methods.
This is a smaller PR which wraps
Graphic3d_MaterialAspectXCAFDoc_MaterialandXCAFDoc_VisMaterialfrom OCC and leaves open the possibility of adding inphysicalvisualization properties later.