refactor: add type hinting to data processors - #331
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the queens.data_processors package to add/strengthen type hints across multiple concrete DataProcessor implementations, and updates tooling configuration so these modules are included in static type checking.
Changes:
- Added/updated type annotations and docstrings for
DataProcessorand several file-based processors (CSV/NumPy/PVD/TXT). - Improved a small piece of TXT parsing robustness (avoid failure when the logger prefix regex doesn’t match a line).
- Updated mypy/pre-commit exclude patterns to stop excluding
src/queens/data_processorsfrom checks.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/queens/data_processors/txt_file.py | Adds type hints and makes logger-prefix stripping resilient to non-matching lines. |
| src/queens/data_processors/pvd_file.py | Adds type hints for PVD reading/filtering and Path-like parameters. |
| src/queens/data_processors/numpy_file.py | Adds type hints for NumPy loading and Path-like parameters. |
| src/queens/data_processors/csv_file.py | Adds type hints and refactors filtering variable names; introduces stricter filter config typing. |
| src/queens/data_processors/_data_processor.py | Adds type hints to the abstract base processor and helper methods. |
| src/queens/data_processors/init.py | Adds typing improvements to lazy-import __getattr__ and postpones annotation evaluation. |
| pyproject.toml | Adjusts mypy exclude list to include data_processors in type checking. |
| .pre-commit-config.yaml | Keeps pre-commit’s mypy exclude list consistent with pyproject (include data_processors). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Nice, type hinting makes coding so much nicer! I can look at it tomorrow probably. In the meantime, I think we have to be careful here with the merge order of #323: either you or me need to incorporate the changes the other PR made. |
Very good point! I think we should merge #323 first, so it doesn't become even bigger. @queens-py/developers, could anyone take another look at #323 and review it? |
leahaeusel
left a comment
There was a problem hiding this comment.
Thank you very much for reviewing this extensive PR! I will also add the missing override decorators shortly.
0b4f49b to
c5ec27b
Compare
ff53dca to
7d2f151
Compare
Description and Context:
What and Why?
This PR adds type hinting to the data processors.
Related Issues and Pull Requests
Interested Parties