ENH add types for dataset classes (BrainData, Dataview, ...) - #669
Merged
Conversation
kroq-gar78
force-pushed
the
types/05-dataset-core
branch
from
August 18, 2026 08:49
50911ec to
a813aad
Compare
kroq-gar78
force-pushed
the
types/05-dataset-core
branch
from
August 20, 2026 20:05
a813aad to
b7c0186
Compare
mvdoc
force-pushed
the
types/05-dataset-core
branch
from
August 20, 2026 20:38
b7c0186 to
b56ca41
Compare
Contributor
Author
|
I think |
kroq-gar78
marked this pull request as ready for review
August 20, 2026 20:55
Base automatically changed from
types/01-03-04-infra-geometry-database
to
main
August 20, 2026 20:55
mvdoc
force-pushed
the
types/05-dataset-core
branch
from
August 20, 2026 20:55
b56ca41 to
d6a8710
Compare
mvdoc
reviewed
Aug 20, 2026
mvdoc
left a comment
mvdoc
left a comment
Contributor
There was a problem hiding this comment.
Just a couple of comments @kroq-gar78
kroq-gar78
force-pushed
the
types/05-dataset-core
branch
2 times, most recently
from
August 20, 2026 21:43
f2c8170 to
5afd170
Compare
The mapper/mapper.py and webgl/data.py/webgl/view.py hunks of this original commit are owned by PR 6 and PR 9 respectively.
Consolidates the rest of PR 5's per-commit typing work (dataset/braindata.py, dataset/views.py, dataset/view2D.py, dataset/viewRGB.py, dataset/__init__.py, tests/test_dataset.py) into one commit rather than replaying the remaining ~15 original commits individually. Reason for the shortcut: several of these commits touch the same lines in a different real chronological order than PR_SPLIT_PLAN.md's "PR-internal commit order is chronological" claim assumes (e.g. `1d2164b5`, dated 2026-03-07 17:54, resolves ColormapDict's `cmap` TODO before `90acd0bd`, dated 2026-03-07 21:29, removes the now-unused ListedColormap import — but the plan lists 90acd0b at position 9 and 1d2164b at position 21). Replaying in the plan's listed order applies real hunks against a file shape they weren't authored against, which is how a silent bug (re-deleting an import still in use) almost landed in the first cherry-pick pass. This commit instead checks out the exact final state of each PR-5-owned file from `types-easy`, verified to produce zero diff against `types-easy` for every file in PR 5's file list.
kroq-gar78
force-pushed
the
types/05-dataset-core
branch
from
August 20, 2026 21:47
5afd170 to
375ac57
Compare
…k overload merge is now keyword-only (*, merge: ...) in every overload, not just the Literal[True] one, closing the gap the "forcing '*' WILL cause issues" TODO was flagging -- no call site in the repo passes merge positionally, so this is a no-op for real callers and just removes the inconsistency. TODO removed. Also adds a fallthrough overload for non-literal hemi/merge callers, needed because Database.get_surf forwards to self.auxfile.get_surf(subject, type, hemisphere, merge=merge, nudge=nudge) with dynamic (non-literal) arguments that couldn't match any of the three literal-narrowed overloads -- confirmed by mypy: database.py:536's "incompatible type" error on this call is now gone, with no new errors introduced.
kroq-gar78
force-pushed
the
types/05-dataset-core
branch
from
August 21, 2026 00:23
defba84 to
eed5da5
Compare
mvdoc
approved these changes
Aug 21, 2026
mvdoc
added a commit
to mvdoc/pycortex
that referenced
this pull request
Aug 21, 2026
* origin/main: (44 commits) WebGL viewer: make opacity slider and `o` toggle work for Vertex data (gallantlab#685) docs(mapper): add get_mapper docstring (gallantlab#686) ENH add types for dataset classes (BrainData, Dataview, ...) (gallantlab#669) Allow viewer data to be updated in real-time (gallantlab#675) FIX: fix minor bug in handling of vmin and vmax when not specified (replaces old cast code) (gallantlab#681) docs: correct the sulcus install instructions, which destroyed existing sulci (gallantlab#657) WebGL viewer: fix help-menu shortcut display (gallantlab#642) MNT ignore docs build outputs written into the source tree (gallantlab#680) DOC fix build warnings (gallantlab#673) webgl: bind an OS-assigned ephemeral port for the viewer server (gallantlab#676) ENH add types for Database and transforms (gallantlab#667) MNT remove obsolete Python 2 syntax, add some types (gallantlab#666) Bump JamesIves/github-pages-deploy-action from 4.8.0 to 4.9.0 (gallantlab#671) Bump actions/setup-python from 6 to 7 (gallantlab#661) docs: document in-browser sulcus drawing (pycortex-roidraw v0.4.0) (gallantlab#656) ENH Pure-python surf2surf matrix (direct nnfr construction) (gallantlab#651) Bump actions/cache from 5 to 6 (gallantlab#654) webgl: fix overlay/label async texture-bake races (complete) (gallantlab#653) docs: add in-browser ROI drawing (pycortex-roidraw) page (gallantlab#652) FIX: update BuWtRd and BuWtRd_alpha colormaps to have pure white at center, more sensible for biphasic data (gallantlab#649) ... # Conflicts: # cortex/quickflat/composite.py # cortex/utils.py # cortex/webgl/resources/js/shaderlib.js # cortex/webgl/view.py
alexhuth
pushed a commit
that referenced
this pull request
Aug 21, 2026
* ColormapDict return type * Add more types to datasets. Add view-specific JSON type for webgl * Monkeytype missing things in Dataset * Typing for Dataset.get_surf * Typing: masker and _hdf* functions in dataset.braindata * cortex.dataset.dataset: more basic annots * cortex/dataset/braindata.py: types for emtpy and random * dataset: introduce DatasetLike type (extracted from a50736f) The mapper/mapper.py and webgl/data.py/webgl/view.py hunks of this original commit are owned by PR 6 and PR 9 respectively. * dataset: remaining Dataset/BrainData/Views typing (PR 5) Consolidates the rest of PR 5's per-commit typing work (dataset/braindata.py, dataset/views.py, dataset/view2D.py, dataset/viewRGB.py, dataset/__init__.py, tests/test_dataset.py) into one commit rather than replaying the remaining ~15 original commits individually. Reason for the shortcut: several of these commits touch the same lines in a different real chronological order than PR_SPLIT_PLAN.md's "PR-internal commit order is chronological" claim assumes (e.g. `1d2164b5`, dated 2026-03-07 17:54, resolves ColormapDict's `cmap` TODO before `90acd0bd`, dated 2026-03-07 21:29, removes the now-unused ListedColormap import — but the plan lists 90acd0b at position 9 and 1d2164b at position 21). Replaying in the plan's listed order applies real hunks against a file shape they weren't authored against, which is how a silent bug (re-deleting an import still in use) almost landed in the first cherry-pick pass. This commit instead checks out the exact final state of each PR-5-owned file from `types-easy`, verified to produce zero diff against `types-easy` for every file in PR 5's file list. * Cleanup (addressing Matteo's comments) * Use forward declaraion for Dataset * dataset: make get_surf's merge keyword-only, add dynamic-hemi fallback overload merge is now keyword-only (*, merge: ...) in every overload, not just the Literal[True] one, closing the gap the "forcing '*' WILL cause issues" TODO was flagging -- no call site in the repo passes merge positionally, so this is a no-op for real callers and just removes the inconsistency. TODO removed. Also adds a fallthrough overload for non-literal hemi/merge callers, needed because Database.get_surf forwards to self.auxfile.get_surf(subject, type, hemisphere, merge=merge, nudge=nudge) with dynamic (non-literal) arguments that couldn't match any of the three literal-narrowed overloads -- confirmed by mypy: database.py:536's "incompatible type" error on this call is now gone, with no new errors introduced. --------- Co-authored-by: Aditya Vaidya <kroq-gar78@users.noreply.github.com>
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
2 participants
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.
This PR adds partial type support for the dataset classes. A rework of the dataset class hierarchy is likely necessary for full type support, but this PR should still be useful for end users.
This makes no behavior changes besides adding some type guards (which would have triggered runtime errors anyway).
Stack created with GitHub Stacks CLI • Give Feedback 💬