refactor: introduce typed simulation state and improve type safety - #11
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The mypy configuration changes are inconsistent with the declared supported Python version range, and the README updates introduce platform-specific commands without Windows equivalents despite claiming Windows support.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR refactors and tightens typing across the DSP and UI layers, while updating developer-facing workflows (type-checking configuration and README instructions) to better support maintenance and static analysis.
Changes:
- Adds explicit typing for FFT reconstruction buffers in DSP utilities.
- Adjusts Matplotlib callback type annotations to match widget callback signatures.
- Updates mypy configuration and expands README guidance for virtual environments, testing, and running the app.
File summaries
| File | Description |
|---|---|
src/aliasing_atlas/dsp.py |
Adds an explicit ndarray annotation for the zero-padding FFT reconstruction buffer. |
src/aliasing_atlas/app.py |
Updates callback parameter types for Matplotlib widget callbacks and guards against None labels. |
README.md |
Revises setup/run/test instructions to use a local virtual environment and module entrypoint. |
pyproject.toml |
Updates mypy settings intended to align type-checking with the current toolchain. |
Review details
Suppressed comments (2)
pyproject.toml:79
- Setting follow_imports = "skip" significantly reduces mypy’s ability to type-check dependencies (including internal modules when checking entry points), which runs counter to the PR’s type-safety goal; ignore_missing_imports already handles missing third-party stubs.
ignore_missing_imports = true
no_site_packages = true
follow_imports = "skip"
check_untyped_defs = true
README.md:130
- The activation command is POSIX-specific; add Windows activation commands to match the project’s stated Windows support.
source .venv/bin/activate
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| git clone https://github.com/Boussetta/NyquistNavigator.git | ||
| cd NyquistNavigator | ||
| python3 -m pip install -e . | ||
| python3 -m venv .venv | ||
| .venv/bin/python -m pip install --upgrade pip | ||
| .venv/bin/python -m pip install -e . |
There was a problem hiding this comment.
Implemented in commit 634fb73 by adding explicit Windows PowerShell equivalents for the .venv/bin/python and activation commands in the installation section of README.md.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Boussetta <4375228+Boussetta@users.noreply.github.com>
Closes #4
Summary
Improve type safety and maintainability across the simulation and GUI layers.
Changes
Validation
pytest -q: 19 passedruff check src tests: passedmypy src/aliasing_atlas --ignore-missing-imports: passedgit diff --check: passedNotes
This PR contains four focused commits: