Skip to content

[Do Not Merge] Support divergent color ramps #912

Draft
nakul-py wants to merge 78 commits intogeojupyter:mainfrom
nakul-py:divergent
Draft

[Do Not Merge] Support divergent color ramps #912
nakul-py wants to merge 78 commits intogeojupyter:mainfrom
nakul-py:divergent

Conversation

@nakul-py
Copy link
Collaborator

@nakul-py nakul-py commented Aug 29, 2025

Description

Addresses #881

  • Currently, Color Ramp Type only supports Divergent colors such as balance, delta , curl, diff and tarn so other colormaps are still show type unknown. Enable users to set a min and max value and also get self-populated min max values for the colormap.
  • Also updates Color Ramp when user setted to reverse.

Checklist

  • PR has a descriptive title and content.
  • PR description contains references to any issues the PR resolves, e.g. Resolves #XXX.
  • PR has one of the labels: documentation, bug, enhancement, feature, maintenance
  • Checks are passing.
    Failing lint checks can be resolved with:
    • pre-commit run --all-files
    • jlpm run lint

📚 Documentation preview: https://jupytergis--912.org.readthedocs.build/en/912/
💡 JupyterLite preview: https://jupytergis--912.org.readthedocs.build/en/912/lite

@github-actions
Copy link
Contributor

Binder 👈 Launch a Binder on branch nakul-py/jupytergis/divergent

@github-actions
Copy link
Contributor

github-actions bot commented Aug 29, 2025

Integration tests report: appsharing.space

@nakul-py nakul-py changed the title Wip: Supporting divergent colomaps with critical value Supporting divergent colomaps with critical value Sep 2, 2025
@nakul-py nakul-py marked this pull request as ready for review September 2, 2025 12:16
Copy link
Collaborator Author

@nakul-py nakul-py left a comment

Choose a reason for hiding this comment

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

cmocean's "balance" is cool :)

@mfisher87 mfisher87 added the enhancement New feature or request label Sep 3, 2025
Copy link
Member

@mfisher87 mfisher87 left a comment

Choose a reason for hiding this comment

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

Hey Nakul! This looks really good so far! I have some thoughts on the implementation details, most importantly around saving the min & max into the project file as symbologyState.min, symbologyState.max and updating singleband pseudocolor (for raster data) to share the same behavior. We can definitely share a common component!

@mfisher87 mfisher87 changed the title Supporting divergent colomaps with critical value Support divergent color ramps Sep 3, 2025
@mfisher87 mfisher87 requested a review from arjxn-py September 3, 2025 16:13
@nakul-py nakul-py marked this pull request as draft September 5, 2025 08:53
@nakul-py nakul-py marked this pull request as ready for review September 9, 2025 11:02
@nakul-py nakul-py requested a review from mfisher87 September 9, 2025 11:02
@mfisher87
Copy link
Member

Hey @nakul-py I'm a bit overwhelmed with interviewing folks for an internship program. I may be a bit delayed!

Co-authored-by: Nakul Verma <nakulverma.py@gmail.com>
Co-authored-by: Nakul Verma <nakulverma.py@gmail.com>
Copy link
Member

@mfisher87 mfisher87 left a comment

Choose a reason for hiding this comment

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

Nakul and I worked on this together for a little bit today. Just leaving some thoughts we talked about -- I'll get to a more comprehensive review on Monday!

These are different things! The data minimum and maximum are attributes
of the data, and symbology min/max are user preference.
@mfisher87
Copy link
Member

Looks like we have a bug where changing the band number in the singleband symbology menu doesn't change the rendered output. The identify tool indicates that we should see a difference between the bands!

let dataMax = image.fileDirectory.STATISTICS_MAXIMUM;

if (dataMin === undefined || dataMax === undefined) {
// 2. Try smallest overview if available
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// 2. Try smallest overview if available
// 2. Try smallest overview if available
// A consequence of this method is that we will almost never find the **true** minimum and maximum for the whole raster, as overviews are aggregate data, averaging groups of grid cells to calculate lower-resolution grid cells. However, calculating the minimum and maximum from the entire dataset may be prohibitively expensive on CPU and memory!

@@ -14,15 +18,28 @@ interface IColorRampProps {
numberOfShades: string,
selectedRamp: string,
setIsLoading: (isLoading: boolean) => void,
criticalValue?: number,
Copy link
Member

Choose a reason for hiding this comment

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

Since this is for display only, classifyFunc doesn't need to receive it as an argument anymore. Let's remove it!

Suggested change
criticalValue?: number,

// Typeguard: This should never happen
return;
}
const scaledCritical =
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const scaledCritical =
// Get the critical value (divergence point in a divergent colormap) as a data value using equal interval interpolation.
// This is only used if equal interval classification mode is selected.
const criticalDataValue =

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I removed the code related to critical value because as we removed critical value also from classification function. So not the const criticalDataValue = used nowhere.

layerParams.symbologyState = {
...layerParams.symbologyState,
dataMin,
dataMax,
Copy link
Member

@mfisher87 mfisher87 Nov 19, 2025

Choose a reason for hiding this comment

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

dataMin and dataMax aren't being saved to the symbology state (we can see that the JSON isn't being updated). This is because we need to call a function to update the shared model; if we only assign to it, it won't update the model. I don't have that function name at hand and have to jump on to a meeting. Let me know if you have trouble finding it!

Though in my testing, things seem to be working well, so I wonder if this whole effect can be deleted and things will still work?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure we can delete specific useEffet.

@mfisher87
Copy link
Member

mfisher87 commented Feb 13, 2026

Nakul and I are on a call together thinking about how we can break this PR in to smaller pieces to make it more reviewable and make it easier to add divergent colormaps in the future. Ideas:

  1. Refactor useGetBandInfo into multi- and single- band versions. This will enhance performance by avoiding analyzing all bands when it's not required. See: 79a7885
  2. Refactor reverseRamp to be part of ColorRampControls instead of repeated in multiple components (Graduated, Categorized). Make the color ramp selector display show as reversed when checked. Move the checkbox over to the right so that it's immediately underneath the colormap selector dropdown.
  3. Extract minor type improvements, e.g. using React.Dispatch instead of callable types
  4. Update the project file to store color stops in a way that they can be restored into the UI. Right now, you need to click "Classify" every time you open the symbology dialog to view the color stops. This is because the stops are stored in the project file in 0-1 space instead of data space. We can store the color stops in data space and then convert too 0-1 space before handing the information off to OpenLayers if need be. Already in progress, see: Add support for saving color stops in symbologyState #949
  5. Extract color ramps to colorRamps.ts and give them types, and display the type in the dropdoown -- don't worry about divergent for now. E.g.:
export interface ISequentialColorRampDefinition
  extends IBaseColorRampDefinition {
  type: 'Sequential';
}

/* Later!
export interface IDivergentColorRampDefinition
  extends IBaseColorRampDefinition {
  type: 'Divergent';
  criticalValue: number;
} */

export interface ICyclicColorRampDefinition extends IBaseColorRampDefinition {
  type: 'Cyclic';
}
export type IColorRampDefinition =
  | ISequentialColorRampDefinition
  | IDivergentColorRampDefinition
  | ICyclicColorRampDefinition;

@nakul-py nakul-py changed the title Support divergent color ramps [Do Not Merge] Support divergent color ramps Feb 25, 2026
@nakul-py nakul-py marked this pull request as draft February 25, 2026 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants