-
Notifications
You must be signed in to change notification settings - Fork 27
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
Do not access private _namedtuple
member from pyvista
#1219
Do not access private _namedtuple
member from pyvista
#1219
Conversation
pre-commit.ci autofix |
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
@all-contributors please add @user27182 for code |
I've put up a pull request to add @user27182! 🎉 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1219 +/- ##
=======================================
Coverage 91.34% 91.34%
=======================================
Files 58 58
Lines 2958 2959 +1
=======================================
+ Hits 2702 2703 +1
Misses 256 256 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
@@ -375,7 +376,9 @@ def combine( | |||
common_point_data = set(first.point_data.keys()) | |||
common_cell_data = set(first.cell_data.keys()) | |||
common_field_data = set(first.field_data.keys()) | |||
active_scalars_info = {first.active_scalars_info._namedtuple} # noqa: SLF001 | |||
active_scalars_info = { | |||
pv.core.dataset.ActiveArrayInfoTuple(*first.active_scalars_info) |
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 pyvista/pyvista#6835 is accepted and the next GeoVista release pins pyvista>=0.45
, then active_scalars_info
is a named tuple and no unpacking is needed. Maybe this change could be made in a future PR?
pv.core.dataset.ActiveArrayInfoTuple(*first.active_scalars_info) | |
first.active_scalars_info |
EDIT: if you don't want to pin PyVista you can branch this instead to support legacy behaviour
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.
@user27182 Awesome, thanks.
I stumbled across my use of the _namedtuple
the other day, thanks to ruff
, and questioned my decision about using it.
Amazing effort with the typing
on pyvista
BTW 💯 🚀
I'll add a changelog news fragment post merge for you to cover this change and give you credit 👍
I'll also create an issue to refactor after the 0.45 release of pyvista
👍
🤟 Congrats on banking your first |
🚀 Pull Request
Description
Remove access to a private variable from
pyvista
(_namedtuple
is removed in pyvista/pyvista#6835, causing GeoVista tests to fail)