Add features to plotting.surface_plotting and plotting.colormaps by rcruces · Pull Request #91 · MICA-MNI/BrainSpace

rcruces · 2023-02-02T23:19:55Z

added views to the default views of surface_plotting
added colormaps

ReinderVosDeWael · 2023-02-10T19:13:37Z

Love the change to add other views, although this would also need to be updated in the docstrings.

I'm not sure I see the value of these additional colormaps. What's the use-case for gradients? As gradients are continuous I'd see little use to adding discrete colormaps, and the good people over at matplotlib have already provided every colormap under the sun including an analysis of their pros/cons. I'd prefer to rely on those rather than risking the feature bloat / additional maintenance. If it's for ease of access within MICA tools wouldn't BrainStat be the more appropriate toolbox as its more of a swiss army-knife?

Copilot

Pull request overview

This PR expands BrainSpace’s plotting utilities by adding new preset camera orientations for surface plotting and bundling additional built-in discrete/continuous colormaps for use via the existing cmap/colormaps lookup.

Changes:

Added new view orientations (anterior, posterior, flatL, flatR) to surface_plotting’s orientation presets.
Added several new built-in colormap tables (eco_kos, spec_5, fs, BuGyRd) to the plotting colormap registry.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.

File	Description
`brainspace/plotting/surface_plotting.py`	Extends the `orientations` preset mapping with additional view options used by `build_plotter`/`plot_surf`.
`brainspace/plotting/colormaps.py`	Adds new RGBA colormap tables and registers them in the module-level `colormaps` dict.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-09T15:41:04Z

+eco_kos_colors = np.array([[0, 0, 0, 255],
+                           [126, 40, 127, 255],
+                           [51, 104, 156, 255],
+                           [167, 210, 140, 255],
+                           [254, 205, 8, 255],


The new colormap arrays don’t document what they represent (source/expected label set/continuous vs discrete). Adding short comments (e.g., provenance and intended number of labels/colors) above each definition would make this file easier to maintain and help users pick the right map.

Copilot · 2026-02-09T15:41:04Z

+fs_colours2 = np.array([[ 0, 0,  0, 255],
+                        [ 25, 100,  40, 255],
+                        [125, 100, 160, 255],
+                        [100,  25,   0, 255],
+                        [220,  20, 100, 255],


fs_colours2 uses inconsistent spelling compared to the existing *_colors variables in this module (e.g., yeo7_colors). Rename to fs_colors2 (and update the colormaps dict accordingly) to keep naming consistent.

Copilot · 2026-02-09T15:41:04Z

+BuGyRd = np.array([[33, 113, 181, 255],
+[35, 115, 182, 255],
+[37, 117, 183, 255],
+[39, 119, 184, 255],
+[41, 121, 185, 255],


BuGyRd is defined with mixed naming style (CamelCase) and without the _colors suffix used by the other colormaps. Consider renaming to something like bu_gyrd_colors (and using a consistently-cased key in colormaps) to keep the public colormap names predictable.

Copilot · 2026-02-09T15:41:05Z

+[35, 115, 182, 255],
+[37, 117, 183, 255],
+[39, 119, 184, 255],
+[41, 121, 185, 255],
+[43, 123, 186, 255],
+[45, 125, 187, 255],
+[47, 127, 188, 255],
+[49, 129, 189, 255],
+[51, 131, 190, 255],
+[53, 133, 191, 255],
+[55, 135, 192, 255],
+[57, 137, 193, 255],
+[59, 139, 194, 255],
+[61, 141, 195, 255],
+[64, 144, 197, 255],
+[66, 146, 198, 255],
+[68, 147, 199, 255],
+[71, 149, 200, 255],
+[73, 151, 201, 255],
+[76, 153, 202, 255],
+[79, 154, 203, 255],
+[81, 156, 204, 255],
+[84, 158, 205, 255],
+[86, 160, 206, 255],


The BuGyRd array formatting is inconsistent with the earlier arrays (elements are not indented/aligned), which makes it hard to read and review. Reformat this block to match the style used for yeo7_colors/fs_* (consistent indentation and alignment).

Suggested change

[35, 115, 182, 255],

[37, 117, 183, 255],

[39, 119, 184, 255],

[41, 121, 185, 255],

[43, 123, 186, 255],

[45, 125, 187, 255],

[47, 127, 188, 255],

[49, 129, 189, 255],

[51, 131, 190, 255],

[53, 133, 191, 255],

[55, 135, 192, 255],

[57, 137, 193, 255],

[59, 139, 194, 255],

[61, 141, 195, 255],

[64, 144, 197, 255],

[66, 146, 198, 255],

[68, 147, 199, 255],

[71, 149, 200, 255],

[73, 151, 201, 255],

[76, 153, 202, 255],

[79, 154, 203, 255],

[81, 156, 204, 255],

[84, 158, 205, 255],

[86, 160, 206, 255],

[35, 115, 182, 255],

[37, 117, 183, 255],

[39, 119, 184, 255],

[41, 121, 185, 255],

[43, 123, 186, 255],

[45, 125, 187, 255],

[47, 127, 188, 255],

[49, 129, 189, 255],

[51, 131, 190, 255],

[53, 133, 191, 255],

[55, 135, 192, 255],

[57, 137, 193, 255],

[59, 139, 194, 255],

[61, 141, 195, 255],

[64, 144, 197, 255],

[66, 146, 198, 255],

[68, 147, 199, 255],

[71, 149, 200, 255],

[73, 151, 201, 255],

[76, 153, 202, 255],

[79, 154, 203, 255],

[81, 156, 204, 255],

[84, 158, 205, 255],

[86, 160, 206, 255],

Copilot · 2026-02-09T15:41:05Z

+colormaps = {'yeo7': yeo7_colors, 'BuGyRd': BuGyRd,
+             'eco_kos': eco_kos_colors, 'spec_5': spec_5_colors, 'fs': fs_colours2 }


New built-in colormaps are added here but there’s no test coverage ensuring plot_surf can resolve these keys via the colormaps dict. Add a lightweight plotting test that uses one of the new keys (e.g., 'eco_kos' or 'BuGyRd') so failures show up quickly if the dict changes.

Copilot · 2026-02-09T15:41:05Z

+                'dorsal': (0, 0, 0),
+                'anterior': (90, 90, 90),
+                'posterior': (270, 90, 90),
+                'flatL': (0, 180, 90),
+                'flatR': (0, 180, 270)}


The docstrings for build_plotter and plot_surf still list possible view values as only {'lateral','medial','ventral','dorsal'}, but this change adds additional supported views (anterior/posterior/flatL/flatR). Update the docstrings to include the new options so users can discover them and the documentation stays accurate.

Copilot · 2026-02-09T15:41:05Z

+                'flatL': (0, 180, 90),
+                'flatR': (0, 180, 270)}


New view keys flatL/flatR introduce mixed casing while all existing view keys are lowercase. Consider renaming these to a consistent lowercase/snake_case form (and optionally keeping backward-compatible aliases) to avoid surprising, case-sensitive API behavior.

Suggested change

'flatL': (0, 180, 90),

'flatR': (0, 180, 270)}

'flat_l': (0, 180, 90),

'flat_r': (0, 180, 270)}

Copilot · 2026-02-09T15:41:06Z

+                'anterior': (90, 90, 90),
+                'posterior': (270, 90, 90),
+                'flatL': (0, 180, 90),
+                'flatR': (0, 180, 270)}


These new view orientations are user-facing behavior but aren’t covered by plotting tests. Adding a small unit test that calls build_plotter/plot_surf with one of the new view strings (e.g., 'anterior') would prevent regressions (e.g., typos in the orientations dict).

zihuaihuai

Thanks for the contribution — extra views and curated colormaps are both useful additions. A few things to resolve before this can merge (this PR is two years old, so flagging in case you can revisit):

Blockers

fs_colours2 licensing. The 35-entry palette looks derived from FreeSurfer's FreeSurferColorLUT.txt. FreeSurfer is licensed under a custom non-commercial license that's not BSD-compatible. Please confirm the source and either credit it explicitly with a link / license note, or rebuild the palette from a BSD-/MIT-/public-domain source.
BuGyRd is 256 hand-listed RGB tuples (≈260 lines). If this is a recoloring of an existing scientific colormap (Spectral, RdBu, cmocean.curl, …), please derive it programmatically — e.g. interpolate between 5–10 control points with np.linspace and return the lookup table. That's much easier to verify and review. If it really is a custom palette, link the source.
Naming inconsistency. Existing code uses _colors suffix (yeo7_colors); this PR adds both _colors (eco_kos_colors, spec_5_colors) and bare names (fs_colours2, BuGyRd). Pick one — _colors matches what's already there.

Smaller items

flatL / flatR views ((0, 180, 90) / (0, 180, 270)). Flat-map views typically need a flattened surface mesh, not just a camera angle. If these orientations only do something useful when the input is pre-flattened, please document that on the orientations dict; otherwise users will report "the view looks identical to ventral".
No tests, no docs. New colormap names should appear in docs/python_doc/api_doc/brainspace.plotting.rst so they're discoverable, and a smoke test (from brainspace.plotting.colormaps import colormaps; assert 'BuGyRd' in colormaps and colormaps['BuGyRd'].shape[1] == 4) would catch accidental breakage.

If you'd prefer, splitting this into "new orientations" (small, easy to merge) and "new colormaps" (needs licensing/sourcing review) would unblock the orientations half immediately.

Subset of #91 — takes the cleanly-licensed parts: - New view orientations in plotting.surface_plotting: anterior, posterior, flatL, flatR. Adds an inline note that flatL/flatR assume a pre-flattened mesh (camera angle alone doesn't flatten geometry). - New small categorical palettes eco_kos and spec_5 (ColorBrewer 5-step Spectral, Apache 2.0). - BuGyRd diverging colormap, but built programmatically by interpolating 5 control points with np.interp instead of hand-listing 256 RGB tuples. Same visual result, 4 lines instead of 256. Skipped from the original PR pending separate clarification: - fs_colours2 (FreeSurfer-derived 35-entry palette): non-BSD source license needs explicit credit/clearance before merging. Co-authored-by: Raul Cruces <rcruces@users.noreply.github.com>

) The remaining unmerged piece of #91 was fs_colours2, a 35-entry palette copied from FreeSurfer's color LUT. FreeSurfer's license is non-commercial only, incompatible with BSD-3, so the palette could not ship as-is. Add cat35 instead: same 35-entry size (matches Desikan-Killiany cortical parcellation: 34 labels + 1 unknown/medial-wall slot), built deterministically from matplotlib's tab20 + tab20b qualitative colormaps (PSF/BSD license). Construction lives in _build_cat35() so the source is auditable instead of hand-listed. Tests verify the registration, shape, opaque alpha, distinct entries, and the leading black slot.

zihuaihuai · 2026-05-04T17:29:42Z

Closing — every part of this PR has now been delivered:

Views (anterior/posterior/flatL/flatR) → merged in Add anterior/posterior/flat views and small colormaps (#91) #158
eco_kos and spec_5 palettes → merged in Add anterior/posterior/flat views and small colormaps (#91) #158
BuGyRd → merged in Add anterior/posterior/flat views and small colormaps (#91) #158, rebuilt programmatically from 5 control points instead of 256 hand-listed rows (visually identical)
fs_colours2 → replaced by cat35 in Add cat35 categorical palette (BSD-compatible substitute for fs_colours2 from #91) #160. Same 35-entry shape, built deterministically from matplotlib's tab20+tab20b (BSD-compatible) since FreeSurfer's color LUT license isn't compatible with BSD-3.

You're credited as Co-author on #158. Thanks for the contribution!

rcruces added 2 commits February 2, 2023 18:15

ADD views anterior, posterior, flatL and flatR

b4fec27

ADD colormaps BuGyRd eco_kos spec_5 and fs

3ac7f9a

rcruces requested review from OualidBenkarim and ReinderVosDeWael as code owners February 2, 2023 23:19

zihuaihuai requested review from Copilot and removed request for OualidBenkarim February 9, 2026 15:35

Copilot started reviewing on behalf of zihuaihuai February 9, 2026 15:35 View session

Copilot AI reviewed Feb 9, 2026

View reviewed changes

zihuaihuai reviewed May 4, 2026

View reviewed changes

zihuaihuai mentioned this pull request May 4, 2026

Add anterior/posterior/flat views and small colormaps (#91) #158

Merged

2 tasks

zihuaihuai mentioned this pull request May 4, 2026

Add cat35 categorical palette (BSD-compatible substitute for fs_colours2 from #91) #160

Merged

2 tasks

zihuaihuai closed this May 4, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add features to plotting.surface_plotting and plotting.colormaps#91

Add features to plotting.surface_plotting and plotting.colormaps#91
rcruces wants to merge 2 commits into
MICA-MNI:masterfrom
rcruces:master

rcruces commented Feb 2, 2023

Uh oh!

ReinderVosDeWael commented Feb 10, 2023

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

zihuaihuai left a comment

Uh oh!

zihuaihuai commented May 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		colormaps = {'yeo7': yeo7_colors, 'BuGyRd': BuGyRd,
		'eco_kos': eco_kos_colors, 'spec_5': spec_5_colors, 'fs': fs_colours2 }

Uh oh!

Conversation

rcruces commented Feb 2, 2023

Uh oh!

ReinderVosDeWael commented Feb 10, 2023

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

zihuaihuai left a comment

Choose a reason for hiding this comment

Blockers

Smaller items

Uh oh!

zihuaihuai commented May 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants