feat: refactor Animator and add MeshesContainer.animate - #1997
Conversation
❌ 1 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
- Add MeshesContainer.animate() using extract_sub_mc + merge_meshes server-side workflow. Exposes 'to_render' (MeshedRegion) and optional 'to_render_field' (Field) for coloring, consistent with FieldsContainer.animate. - Rewrite FieldsContainer.animate to use extract_sub_fc + merge_fields + mesh.from_field, exposing the same 'to_render' / 'to_render_field' contract. - Unify animate_workflow in _PyVistaAnimator: remove mode_number/mode_frequency flags; per-frame connect is either a label-space dict (collection path) or a 0-based index list (direct-Animator fallback). Remove dead branches. - Rewrite animate_mode to pre-build a FieldsContainer of amplitude-scaled frames and delegate to FieldsContainer.animate, making mode-shape animation a degenerate case of collection animation and eliminating a bespoke code path. Fixes the bug where scoping IDs were used as scale weights instead of the actual amplitude values. - Add list-length guard for scale_factor in animate_workflow. - Improve animation.py module and animate_mode docstrings: add type hints to signature, remove types from docstring, add Returns/Raises sections, fix type_mode=1 amplitude description (1 -> 0 -> 1, always non-negative). - Extend test_animation.py with save_as, deform_scale_factor, frame_number, and error-path coverage. Fix two broken test_animator.py expectations (NotImplementedError -> ValueError for Field/FC scale_factor types).
bd52a3b to
da605a8
Compare
|
The documentation for this pull request will be available at https://dpf.docs.pyansys.com/pull/1997. Please allow some time for the documentation to be deployed. |
There was a problem hiding this comment.
Pull request overview
This PR refactors the animation pipeline to support animating workflows that yield either Fields or MeshedRegions, and adds a first-class MeshesContainer.animate() API to animate time- (or other label-) varying meshes with optional coloring/deformation.
Changes:
- Added
MeshesContainer.animate()that builds a label-space workflow (extract_sub_mc+merge_meshes) and drives it viaAnimator.animate(...). - Refactored
FieldsContainer.animate()to use the same label-space workflow convention and mesh-mode rendering (with optionalto_render_fieldcoloring). - Enhanced
Animator.animate_workflow()to support label-space inputs and mesh-mode rendering, plus stricterscale_factorlist length validation; updated/added tests accordingly.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_meshescontainer.py | Adds new animation-focused tests for MeshesContainer.animate(). |
| tests/test_animator.py | Updates scale-factor error expectations for animator/FC animations. |
| tests/test_animation.py | Adds additional behavioral tests around animation.animate_mode() and saving GIFs. |
| src/ansys/dpf/core/plotter.py | Prevents index error when plotting overall fields with empty data (fills with NaN). |
| src/ansys/dpf/core/meshes_container.py | Adds MeshesContainer.animate() and updates typing/imports around MeshedRegion. |
| src/ansys/dpf/core/fields_container.py | Refactors FC animation to label-based iteration + mesh-mode rendering; adds label parameter. |
| src/ansys/dpf/core/animator.py | Adds mesh-mode + label-space support to workflow animation; validates scale-factor list length. |
| src/ansys/dpf/core/animation.py | Refactors animate_mode() to build a scaled FC and delegate to FieldsContainer.animate(). |
Comments suppressed due to low confidence (1)
src/ansys/dpf/core/animator.py:113
Animator.animateand other call sites typescale_factorasSequence[float], butanimate_workflowonly acceptslistexplicitly (type(scale_factor) == list). This will unexpectedly reject tuples, numpy arrays, etc. Consider usingisinstance(scale_factor, Sequence)(excludingstr/bytes) and converting to a list, then performing the length check.
type_scale = type(scale_factor)
if type_scale in [int, float]:
scale_factor = [float(scale_factor)] * len(indices)
elif type_scale == list:
if len(scale_factor) != len(indices):
raise ValueError(
f"The scale_factor list length ({len(scale_factor)}) must match the "
f"number of animation frames ({len(indices)})."
)
else:
raise ValueError(
"Argument scale_factor must be an int, a float, or a list of either, "
f"(not {type_scale})"
)
…st_animator.py Agent-Logs-Url: https://github.com/ansys/pydpf-core/sessions/86e7ae6e-8d64-4f63-a83c-38ebf04038ca Co-authored-by: PProfizi <100710998+PProfizi@users.noreply.github.com>
Agent-Logs-Url: https://github.com/ansys/pydpf-core/sessions/0af368d3-00d3-402f-9276-45ad7f76de68 Co-authored-by: PProfizi <100710998+PProfizi@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…(0/1) Agent-Logs-Url: https://github.com/ansys/pydpf-core/sessions/51415bf2-68f6-416d-955c-54f42d4aa1ef Co-authored-by: PProfizi <100710998+PProfizi@users.noreply.github.com>
|
@copilot add more tests to cover all lines. |
…ieldsContainer.animate, MeshesContainer.animate Agent-Logs-Url: https://github.com/ansys/pydpf-core/sessions/ddf2a4cf-0db1-4ec5-8d34-0f585ce94875 Co-authored-by: PProfizi <100710998+PProfizi@users.noreply.github.com>
Agent-Logs-Url: https://github.com/ansys/pydpf-core/sessions/ddf2a4cf-0db1-4ec5-8d34-0f585ce94875 Co-authored-by: PProfizi <100710998+PProfizi@users.noreply.github.com>
Added in commits 07ccbb6 and 4fb7a3e. New tests cover:
|
Co-authored-by: Paul Profizi <100710998+PProfizi@users.noreply.github.com>
Following new usage with time-varying meshes, this adds the capability to animate a MeshesContainer with a
timelabel.