ENH add type annotations to frontend#597
ENH add type annotations to frontend#597kroq-gar78 wants to merge 20 commits intogallantlab:mainfrom
Conversation
|
Awesome! We are testing on 3.8 and 3.9, but they both reached EOL. You can remove them from the matrix so we can run the tests on CI. |
71ba29d to
9df8f1c
Compare
9df8f1c to
b28dfae
Compare
cortex/dataset/braindata.py
Outdated
| self.dv.copy(self.dv.volume[:, mask].squeeze()) | ||
| # TODO: should be return? | ||
| return self.dv.copy(self.dv.volume[:, mask].squeeze()) |
There was a problem hiding this comment.
I'm actually really not sure what this line was supposed to be doing. Any ideas @mvdoc ?
There was a problem hiding this comment.
It seems broken code to me. If it ever got to that except, then it would blow up because mask is undefined
There was a problem hiding this comment.
Even in the original commit (f4f2fe9) I'm not sure what was supposed to happen... should we just delete the try/except?
There was a problem hiding this comment.
Pull request overview
This PR introduces type annotations across pycortex’s user-facing APIs (notably WebGL viewing, dataset views, and export helpers) to enable better static type checking and provide a foundation for broader internal typing.
Changes:
- Add/expand type annotations across
cortex.dataset, WebGL viewer entry points, and export utilities. - Update packaging/config to support typing work (typing backports, mypy config) and adjust CI Python version matrices.
- Minor modernizations/cleanup (e.g., Python-2-era compatibility branches, NumPy API usage).
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
setup.py |
Changes how install_requires is read from requirements.txt. |
requirements.txt |
Adds typing_extensions conditional dependency. |
pyproject.toml |
Adds PEP 621 [project] metadata stub, mypy settings, and dev dependency group. |
cortex/xfm.py |
Replaces isstr(...) check with isinstance(..., str). |
cortex/webgl/view.py |
Adds typing to show(...) and some internal variables. |
cortex/webgl/serve.py |
Simplifies Queue import logic; adds type annotations to class attrs. |
cortex/webgl/__init__.py |
Updates DocLoader usage to support type checking via TYPE_CHECKING. |
cortex/utils.py |
Makes DocLoader generic (ParamSpec/TypeVar) and adds typed lazy loader for get_mapper. |
cortex/quickflat/view.py |
Adds typing to quickflat public helpers and internal utilities. |
cortex/quickflat/composite.py |
Replaces deprecated-ish np.in1d usage with np.isin. |
cortex/options.py |
Removes legacy Python 2 configparser fallback. |
cortex/export/save_views.py |
Adds typed dicts and function annotations for view export helper. |
cortex/export/panels.py |
Adds typed dicts and function annotations for panel plotting helper. |
cortex/dataset/views.py |
Adds overloads and more explicit typing for core view classes/utilities. |
cortex/dataset/viewRGB.py |
Adds typing and some additional runtime validation for RGB views. |
cortex/dataset/view2D.py |
Adds typing and extra runtime validation for 2D views. |
cortex/dataset/dataset.py |
Adds typing/overloads to dataset normalization and internals. |
cortex/dataset/braindata.py |
Adds typing (Self, NDArray types) and adjusts some return behavior. |
cortex/dataset/__init__.py |
Adds __future__.annotations and exports Dataview2D. |
cortex/__init__.py |
Updates Python-2 version check style. |
.github/workflows/run_tests.yml |
Updates tested Python versions and apt install flags. |
.github/workflows/install_from_wheel.yml |
Updates Python matrix for wheel build/install job. |
.github/workflows/build_docs.yml |
Updates docs build Python version and dependency install steps. |
💡 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.
| angles_and_surfaces = list(set(angles_and_surfaces)) | ||
| list_angles: list[Union[str, tuple[str, ViewParams]]] | ||
| list_surfaces: list[Union[str, ViewParams]] | ||
| list_angles, list_surfaces = list(zip(*angles_and_surfaces)) |
There was a problem hiding this comment.
list_angles/list_surfaces are annotated as list[...], but list(zip(*angles_and_surfaces)) unpacks into tuples. Either convert to lists (e.g., list(...)) after unpacking or annotate them as tuples to keep the types consistent for static checking.
| list_angles, list_surfaces = list(zip(*angles_and_surfaces)) | |
| list_angles, list_surfaces = map(list, zip(*angles_and_surfaces)) |
| base_name: str="fig", | ||
| list_angles: list[Union[str, tuple[str, ViewParams]]]=["lateral_pivot"], | ||
| list_surfaces: list[Union[str, ViewParams]]=["inflated"], | ||
| viewer_params: dict[str, Any]=dict(labels_visible=[], overlays_visible=["rois"]), |
There was a problem hiding this comment.
viewer_params uses a mutable default (dict(...) containing mutable lists). Mutations will be shared across calls. Prefer defaulting to None and creating the dict inside the function.
| viewer_params: dict[str, Any]=dict(labels_visible=[], overlays_visible=["rois"]), | |
| viewer_params: dict[str, Any] | None = None, |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This introduces type annotations to the user-facing API. Several functions, like
cortex.quickshowandcortex.webshow, have numerous arguments with poorly defined inputs. This should now allow explicit type checking in user code.This PR also provides a starting point for type annotations within the library, which should help maintenance.
I believe this also hard breaks support for Python 3.8, but since both 3.8 and 3.9 are EOL anyway, this PR drops support for both.