-
Notifications
You must be signed in to change notification settings - Fork 4
feat: enhance the project preview: irrad, rad, camera sensor features #528
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 ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #528 +/- ##
==========================================
+ Coverage 87.42% 88.05% +0.63%
==========================================
Files 37 38 +1
Lines 3943 4127 +184
==========================================
+ Hits 3447 3634 +187
+ Misses 496 493 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Thanks for taking into account the previous comments @pluAtAnsys
I left some other comments
for more information, see https://pre-commit.ci
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.
There is still a conversation that needs to be handled (currently marked as solved but nothing has been performed to match the request).
Also, quick question: do we need to add the graphics_required decorator for methods that do not directly import pyvista ? Would it be better to delegate that to the inner calls of the functions ? For example, having add_data_triangle
raise the exception through the decorator and not decorating visual_data
?
that is true. I forgot to remove that decorator as now that is handled inside the visualization_methods.py |
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
Co-authored-by: Sébastien Morais <[email protected]>
Description
Please provide a brief description of the changes made in this pull request.
Issue linked
Please mention the issue number or describe the problem this pull request addresses.
Checklist
feat: add optical property
)