Skip to content

ENH add type annotations to frontend#597

Open
kroq-gar78 wants to merge 20 commits intogallantlab:mainfrom
kroq-gar78:frontend-types
Open

ENH add type annotations to frontend#597
kroq-gar78 wants to merge 20 commits intogallantlab:mainfrom
kroq-gar78:frontend-types

Conversation

@kroq-gar78
Copy link
Contributor

@kroq-gar78 kroq-gar78 commented Mar 7, 2026

This introduces type annotations to the user-facing API. Several functions, like cortex.quickshow and cortex.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.

@mvdoc
Copy link
Contributor

mvdoc commented Mar 7, 2026

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.

@kroq-gar78 kroq-gar78 force-pushed the frontend-types branch 2 times, most recently from 71ba29d to 9df8f1c Compare March 7, 2026 10:49
@kroq-gar78 kroq-gar78 marked this pull request as ready for review March 7, 2026 11:20
Comment on lines +627 to +637
self.dv.copy(self.dv.volume[:, mask].squeeze())
# TODO: should be return?
return self.dv.copy(self.dv.volume[:, mask].squeeze())
Copy link
Contributor Author

@kroq-gar78 kroq-gar78 Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm actually really not sure what this line was supposed to be doing. Any ideas @mvdoc ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems broken code to me. If it ever got to that except, then it would blow up because mask is undefined

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even in the original commit (f4f2fe9) I'm not sure what was supposed to happen... should we just delete the try/except?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I would just remove it

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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))
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
list_angles, list_surfaces = list(zip(*angles_and_surfaces))
list_angles, list_surfaces = map(list, zip(*angles_and_surfaces))

Copilot uses AI. Check for mistakes.
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"]),
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
viewer_params: dict[str, Any]=dict(labels_visible=[], overlays_visible=["rois"]),
viewer_params: dict[str, Any] | None = None,

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants