Issue triage: docstring cleanup, test fixes, gvec sync - #58
Conversation
Convert Google-style docstrings (Args/Returns) to NumPy-style (Parameters/Returns with type annotations) across source modules. Remove stale automodule directives from API docs and fix cross-references. Add mesh=None guard in Surface._dimensionalise_coords(). Underworld development team with AI support from Claude Code
Remove xfail markers from three tests that now pass: - test_no_ijk (AttributeError is the expected behavior, not a failure) - test_viscoelastic_plastic_model_tensors (copy() bug resolved) - test_rayleigh_number_is_dimensionless (units propagation working) Skip test_quantities_simplified.py entirely — imports from quantities_simplified module which was never implemented. Underworld development team with AI support from Claude Code
There was a problem hiding this comment.
Pull request overview
This PR focuses on improving correctness of solver output persistence (ensuring solution vectors are properly synced before writing/checkpointing), while also performing a broad documentation/docstring normalization and cleaning up tests and markers.
Changes:
- Ensure solver results are synced to global vectors before write/checkpoint operations so outputs aren’t saved as zeros.
- Standardize docstrings/docs to NumPy-style sections and remove/adjust stale Sphinx API directives and code fences.
- Update tests by removing stale
xfailmarkers and skipping a test module targeting a non-existent API; add aSurfaceguard formesh is None.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_quantities_simplified.py | Skip tests that target a non-existent quantities_simplified module (with explanatory note). |
| tests/test_0850_units_propagation.py | Remove stale xfail marker for Rayleigh-number unit behavior. |
| tests/test_0610_constitutive_tensor_regression.py | Remove stale xfail marker around viscoelastic-plastic tensor regression. |
| tests/test_0601_mesh_vector_calc.py | Remove xfail marker related to .ijk behavior. |
| src/underworld3/utilities/unit_aware_array.py | Condense/standardize class docstring to NumPy-style wording. |
| src/underworld3/utilities/nd_array_callback.py | Condense/standardize class docstring and parameter formatting. |
| src/underworld3/units.py | Convert dimensionalise() docstring to NumPy format and refresh examples. |
| src/underworld3/systems/ddt.py | Update module header docs formatting (class summaries). |
| src/underworld3/swarm.py | Update module header docs formatting; adjust migration_control() examples formatting. |
| src/underworld3/model.py | Convert various docstrings to NumPy sections and reformat content. |
| src/underworld3/meshing/surfaces.py | Convert docstrings to NumPy format; add mesh is None guard in coordinate dimensionalisation; reformat influence_function() docs. |
| src/underworld3/meshing/spherical.py | Update meshing function docstrings to NumPy format (incl. returns/examples). |
| src/underworld3/meshing/geographic.py | Reformat examples/notes sections for MyST/Sphinx rendering. |
| src/underworld3/meshing/faults.py | Convert compute_distance_field() / transfer_normals() docstrings to NumPy format. |
| src/underworld3/meshing/annulus.py | Update SegmentofAnnulus() docstring to NumPy format (incl. returns/examples). |
| src/underworld3/maths/init.py | Reformat module index docs (function/class summaries). |
| src/underworld3/materials.py | Convert docstring section headers to NumPy format (minor formatting tweaks). |
| src/underworld3/function/functions_unit_system.py | Simplify/standardize docstring wording for evaluate()/global_evaluate(). |
| src/underworld3/function/_function.pyx | Reformat UnderworldFunction docstring to clearer NumPy-style sections. |
| src/underworld3/function/init.py | Reformat module index docs and unit-conversion section summary. |
| src/underworld3/discretisation/discretisation_mesh.py | Minor docstring wording/formatting update for checkpoint writing. |
| src/underworld3/discretisation/init.py | Reformat module index docs and API summary bullets. |
| docs/developer/subsystems/constitutive-models-anisotropy.md | Replace citation directives with plain-text references. |
| docs/developer/guides/HOW-TO-WRITE-UW3-SCRIPTS.md | Adjust fenced block language to text for an error snippet. |
| docs/developer/design/WHY_UNITS_NOT_DIMENSIONALITY.md | Adjust fenced block language to text for clarity/robust rendering. |
| docs/developer/design/COORDINATE_MIGRATION_GUIDE.md | Replace emoji-heavy “cheat sheet” code block with a text table-style block. |
| docs/developer/container/README.md | Add MyST front-matter (orphan: true) to suppress orphan warnings. |
| docs/developer/UW3_Developers_MathematicalObjects.md | Adjust fenced block language to text for non-Python pseudo-syntax. |
| docs/beginner/quickstart.md | Fix figure path to correctly reference docs/media. |
| docs/api/systems_ddt.md | Remove stale automodule directive block. |
| docs/api/swarm.md | Remove stale automodule / PopulationControl directive block(s). |
| docs/api/solvers.md | Remove stale automodule directive block. |
| docs/api/maths.md | Remove stale automodule directive block. |
| docs/api/function.md | Update UWexpression autoclass target; remove stale directive blocks/sections. |
| docs/api/discretisation.md | Remove stale automodule directive block. |
| docs/api/coordinates.md | Remove stale automodule directive block. |
| docs/api/constitutive_models.md | Remove stale automodule directive block. |
| docs/advanced/index.md | Add SWARM-INTEGRATION-STATISTICS to the advanced docs toctree. |
Comments suppressed due to low confidence (1)
tests/test_0601_mesh_vector_calc.py:152
test_no_ijk()currently just evaluatesv13.ijkwithout asserting anything. If.ijkis absent, this will error (failing the test); if it exists, the test will pass without verifying the intended behavior. Usepytest.raises(AttributeError)(or equivalent assertion) to ensure the test actually checks that.ijkis not available (or reintroduce xfail if it is still expected to fail).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| name : str, optional | ||
| Human-readable name for this model instance | ||
| **kwargs : dict | ||
| \*\*kwargs : dict |
There was a problem hiding this comment.
The docstring escapes the parameter name as \*\*kwargs, which will render literally in generated docs and is inconsistent with other NumPy-style docstrings in the repo that use **kwargs. Prefer **kwargs : dict (optionally wrapped in double backticks) without backslashes.
| \*\*kwargs : dict | |
| **kwargs : dict |
| ptype : ParameterType, optional | ||
| Parameter type for validation (not used yet) | ||
| **kwargs : dict | ||
| \*\*kwargs : dict |
There was a problem hiding this comment.
Same as above: \*\*kwargs : dict is escaped in the NumPy-style docstring and will likely render with backslashes. Use **kwargs : dict (optionally in double backticks) to match existing docstrings.
| \*\*kwargs : dict | |
| **kwargs : dict |
| ---------- | ||
| callback : callable | ||
| Function called as callback(event_type, *args) | ||
| Function called as callback(event_type, \*args) |
There was a problem hiding this comment.
The docstring currently says callback(event_type, \*args), which will likely render the backslash in built docs. Prefer callback(event_type, *args) and/or wrap the signature in double backticks for consistent rendering.
| Function called as callback(event_type, \*args) | |
| Function called as ``callback(event_type, *args)`` |
| width : float | ||
| Characteristic width of the transition zone. |
There was a problem hiding this comment.
influence_function() converts width via uw.scaling.non_dimensionalise() when it has a .magnitude attribute (i.e., Pint quantities / UnitAwareArray), but the docstring documents width as a plain float. Update the parameter docs to reflect that unit-aware quantities are accepted and will be converted to model coordinates.
| width : float | |
| Characteristic width of the transition zone. | |
| width : float or unit-aware quantity | |
| Characteristic width of the transition zone. If ``width`` has a | |
| ``.magnitude`` attribute (e.g. Pint quantity or ``UnitAwareArray``), | |
| it will be converted to model (non-dimensional) coordinates via | |
| ``uw.scaling.non_dimensionalise``. |
Summary
_gvecnot synced after solver runs — checkpoint/write methods were saving zeros because solution data was only in_lvec. Adds_sync_lvec_to_gvec()called from solver finalization and write methods.Args:/Returns:to NumPy-styleParameters/Returnsacross 34 source and doc files. Clean up stale Sphinx API directives.test_quantities_simplified.py(imports non-existent module).Surface._dimensionalise_coords()no longer crashes whenmesh is None.Test plan
Underworld development team with AI support from Claude Code