Merged
Conversation
…ion and field aliases Phase 2 (Foundation & Code Quality) source changes: - Add [tool.pytest.ini_options] with testpaths, markers, and filterwarnings - Replace hardcoded __version__ with importlib.metadata lookup - Add VTKData field shape validation (ValueError on mismatch) - Add NaN/inf warnings and __repr__ to VTKData - Add FIELD_ALIASES and CANONICAL_NAMES for field name normalization - Add __contains__ and has_field() to VTKData for alias-aware lookup - Normalize SCALARS field names at VTK read time (e.g. "pressure" -> "p")
35 tests for VTK reader covering construction, validation, field aliases, STRUCTURED_POINTS/RECTILINEAR_GRID parsing, malformed files, and the read_vtk_velocity convenience function. 5 integration tests verifying the full pipeline: VTK read -> compute derived fields -> plot, cfd-python conversion, field alias resolution, version accessibility, and VTKData repr.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR completes “Phase 2” foundation work by tightening package/test configuration, improving VTK reader robustness and ergonomics (aliases/validation), and adding substantial automated test coverage.
Changes:
- Adds pytest configuration (testpaths, markers, warning filters) in
pyproject.toml. - Enhances
VTKData/VTK reader with field validation, NaN/inf warnings, alias-aware access, and canonical name normalization at read-time. - Adds a large set of VTK reader unit tests plus end-to-end integration tests, and updates the roadmap status.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
pyproject.toml |
Adds pytest ini options (markers, filterwarnings). |
cfd_viz/common/vtk_reader.py |
Adds canonical naming/aliases, VTKData validation/warnings, repr, alias-aware accessors, and scalar name normalization. |
cfd_viz/common/__init__.py |
Re-exports FIELD_ALIASES. |
cfd_viz/__init__.py |
Switches __version__ to importlib.metadata.version() with fallback. |
tests/common/test_vtk_reader.py |
Adds comprehensive unit tests for the VTK reader and alias behavior. |
tests/common/__init__.py |
Adds package marker for tests.common. |
tests/test_integration.py |
Adds end-to-end integration tests for reading/conversion/plotting/version/repr. |
ROADMAP.md |
Marks Phase 2 as complete and summarizes delivered work. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
VTK sample files are gitignored (*.vtk) so they don't exist in CI. Tests that depend on real VTK files now skip gracefully instead of failing.
…LARS parser The isalpha() check treated valid float tokens like "nan" and "inf" as VTK keywords, prematurely terminating scalar field parsing. Check against known VTK section headers instead.
…ject.toml Replace hard-coded "0.1.0" fallback with "0+unknown" so it cannot silently drift from the canonical version in pyproject.toml.
Tests that previously required gitignored VTK sample files now generate their own test data via _write_structured_points in tmp_path, so they work in CI without needing real data files.
…VTK fixtures Integration tests now generate their own VTK data in tmp_path instead of depending on gitignored sample files, ensuring they work in CI.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Renamed test_canonical_names_not_in_aliases_as_values to test_canonical_names_not_aliased_to_themselves since it asserts canonical names are not present as keys in FIELD_ALIASES.
np.isnan/np.isinf raise TypeError on integer arrays. Skip the check for non-floating dtypes since they cannot contain NaN or inf.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
…ormalization Eliminates duplicate pressure->p mapping by using FIELD_ALIASES as the single source of truth for both read-time normalization and lookup-time aliasing.
Add validate=True parameter to VTKData constructor so callers can skip the O(N) per-field nan/inf scans for large grids when not needed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
importlib.metadata(single source inpyproject.toml)__repr__FIELD_ALIASES,CANONICAL_NAMES) sodata["pressure"]resolves todata["p"]__contains__andhas_field()to VTKData for alias-aware membership testsTest plan
pytest tests/ --ignore=tests/interactive -v— 526 passedruff check .— cleanruff format --check .— clean (pre-existing example formatting excluded)python -c "from cfd_viz import __version__; print(__version__)"— prints0.1.0