-
Notifications
You must be signed in to change notification settings - Fork 441
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
Deprecate pyvista.utilities #4507
Conversation
Codecov Report
@@ Coverage Diff @@
## main #4507 +/- ##
==========================================
+ Coverage 95.71% 95.75% +0.03%
==========================================
Files 107 126 +19
Lines 21066 21094 +28
==========================================
+ Hits 20164 20198 +34
+ Misses 902 896 -6 |
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.
Most of this looks good, why do you feel it's hacky? I think your loop of _try_import
s is a better design than the previous implementation.
Co-authored-by: Andras Deak <adeak@users.noreply.github.com>
Co-authored-by: Andras Deak <adeak@users.noreply.github.com>
Co-authored-by: Andras Deak <adeak@users.noreply.github.com>
Co-authored-by: Andras Deak <adeak@users.noreply.github.com>
Co-authored-by: Andras Deak <adeak@users.noreply.github.com>
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 bearing with me @banesullivan @akaszynski. Looks great to me now.
Always appreciate your attention to detail @adeak! |
Follow up to #4486 to properly deprecate the
pyvista.utilities
module and test that this API is still useable (non-breaking) but does issue deprecation warnings.This whole thing feels hacky, I'm open to suggestions on other ways to do this