Thredds palettes #7059

sixlighthouses · 2024-02-23T07:13:45Z

What this PR does

Adds the functionality to allow users to make use of Thredds palettes for styling.

Test me

Browse to
url: http://ci.terria.io/thredds-palettes/

Add a thredds server -- example https://thredds.ereefs.aims.gov.au/thredds/wms/GBR4_H2p0_B3p1_Cq3P_Dhnd/monthly.nc?service=WMS&version=1.3.0&request=GetCapabilities

Add a layer

Select a style from the workbench control

You will now see the Palettes drop down

select a palette to see the layers style change n.b some layers will need to have the Color Scale Range edited to correctly use the palettes

Checklist

There are unit tests to verify my changes are correct or unit tests aren't applicable (if so, write quick reason why unit tests don't exist)
I've updated relevant documentation in doc/.
I've updated CHANGES.md with what I changed.
I've provided instructions in the PR description on how to test this PR.

…ts and add the fetchPalettes async function to get palettes from styles GetMetadata url

sixlighthouses · 2024-02-25T23:58:18Z

On first load of the Thredds server no style is selected, so no palettes are displayed

Once the user selects a style, we fetch the list of palettes associated with that style

sppigot · 2024-03-08T05:19:41Z

Thanks @sixlighthouses - this is a really useful addition to the THREDDS WMS support! Please merge ASAP

lib/Models/Catalog/Ows/WebMapServiceCapabilitiesStratum.ts

ljowen · 2024-03-14T00:54:12Z

lib/Models/Catalog/Ows/WebMapServiceCapabilitiesStratum.ts

+      //   const paletteResult: StratumFromTraits<WebMapServiceAvailableLayerStylesTraits>[] =
+      // [];
+
+      async () => {


How does this anonymous function get called?

That one is an IIFE -- I used it so I can call the async fetchPalettes without having to make the parent function async

Thanks for picking that one up Lawrence, it was never getting called, removed that code block

lib/Models/Catalog/Ows/WebMapServiceCapabilitiesStratum.ts

Co-authored-by: Lawrence Owen <lawrence.owen@csiro.au>

ljowen · 2024-03-19T23:28:08Z

lib/Models/Catalog/Ows/WebMapServiceCatalogItem.ts

+      const extractedUrl = urls?.[0];
+
+      if (!extractedUrl) {
+        reject("No URL found in abstract");


Unless we also return here the fetch below will still execute and make a request to <ur>/undefined

Should No url in the abstract be treated as an error? My understanding is it's an option property for thredds wms's so it might instead be more appropriate to resolve([])

Updated to resolve, thanks Lawrence

…er than rejecting

nf-s

I think there are a few architectural issues with this PR.

The ncWMS GetMetadata with item=layerDetails (which is currently being extracted from the selected WMS style abstract) - actually has nothing to do with specific styles, it returns extra metadata for a given layer.

For example

https://thredds.ereefs.aims.gov.au/thredds/wms/GBR4_H2p0_B3p1_Cq3P_Dhnd/monthly.nc?request=GetMetadata&item=layerDetails&layerName=CS_bleach

The JSON response includes the following things related to palettes

{
  "scaleRange": [-50, 50],
  "palettes": [
    "default",
    "default-inv",
    "div-BrBG",
    ...
  ],
  "defaultPalette": "psu-viridis",
  "noPaletteStyles": ["contours"]
}

This means that the palettes should be visible if any style is selected that isn't in noPaletteStyles (styles that don't support palettes). The default palette should also be set to defaultPalette, and the default colorScaleMinimum and colorScaleMaximum should be set using scaleRange.

This GetMetadata JSON should be fetched in WebMapServiceCatalogItem.forceLoadMetadata - and should be in it's own LoadableStratum (eg something like NcMWSGetMetadataStratum). From that LoadableStratum, you can define values for threddsPalette and threddsPalettes (and also colorScaleMinimum and colorScaleMaximum).

You can also check the current layer there, and return undefined if the layer name is in noPaletteStyles.

The GetMetadata URL can be constructed using something like

this.uri.clone()
  .setSearch({
    service: "WMS",
    version: this.useWmsVersion130 ? "1.3.0" : "1.1.1",
    request: "GetMetadata",
    item: "layerDetails",
    layerName: this.layersArray[0],
  })
  .toString()

While WMS does support multiple layers, I think we should just fetch GetMetadata for the first layer at the moment.

How you have setup threddsPaletteDimensions works well!
So only minimal changes should be needed for that.

Finally, can you please add a supportsGetMetadata and supportsPalette traits? You can just copy supportsColorScaleRange. This will allow us to override the default behaviour - eg turn it off or on manually.

Also, sorry to nitpick, but all of this is ncWMS not THREDDs specific - so can you please rename the WebMapServiceCatalogItem traits to palette and availablePalettes (just to match the existing pattern with availableStyles etc) - and other thredds name properties/objects

Please let me know if you need any further info/clarification - thanks!

nf-s · 2024-03-28T05:26:23Z

lib/Models/Catalog/Ows/WebMapServiceCatalogItem.ts

@@ -671,6 +738,7 @@ class WebMapServiceCatalogItem
            );
            styles[layerIndex] = newStyle;
            this.setTrait(stratumId, "styles", styles.join(","));
+            this.fetchPalettes();


Can you please add a condition here to fetchPalettes is only called if the server supports it?

Currently, if you select a style for a non-THREDDs WMS - this causes an uncaught error

Sorry this comment doesn't make sense with all the other changes I have suggested

…et the metadata url for each layer

sixlighthouses

added traits for

colorScaleMinimum
colorScaleMaximum
defaultPalette
noPaletteStyles

added checks for current style is included in noPaletteStyles

sixlighthouses

Todo

GetLegendGraphic failing when not using default
GetLegendGraphic being reset when changing styles

sixlighthouses · 2024-04-15T00:23:06Z

Ready for re-review

nf-s

Thanks @sixlighthouses this is definitely a good step forward. There are still a few architectural issues with this PR.

It is important to follow the Terria model layer patterns that I mentioned previously.

If you are unsure how to apply the Terria model layer patterns, there is some documentation here:

Please let me know if you want any further info, we can setup a meeting to discuss this.

nf-s · 2024-05-03T06:37:39Z

lib/Traits/TraitsClasses/WebMapServiceCatalogItemTraits.ts

+    description:
+      "Used to store the palettes available for a layer. This is a non-standard property supported by THREDDS servers. This property is ignored unless WebMapServiceCatalogItem's isThredds is true. The default value is ['default']."
+  })
+  palettes?: string[] = [];


Can you please rename this to availablePalettes (just to match the existing pattern with availableStyles etc)

nf-s · 2024-05-03T07:01:17Z

lib/Models/Catalog/Ows/WebMapServiceCatalogItem.ts

+          this.setTrait(CommonStrata.user, "palettes", data.palettes);
+          this.setTrait(
+            CommonStrata.user,
+            "colorScaleMinimum",
+            data.scaleRange[0]
+          );
+          this.setTrait(
+            CommonStrata.user,
+            "colorScaleMaximum",
+            data.scaleRange[1]
+          );
+          this.setTrait(
+            CommonStrata.user,
+            "defaultPalette",
+            data.defaultPalette
+          );
+          this.setTrait(
+            CommonStrata.user,
+            "noPaletteStyles",
+            data.noPaletteStyles
+          );
+          this.setTrait(CommonStrata.user, "showPalettes", true);
+          resolve(data.palettes);


The user stratum should only be used for user-driven changes to traits. The current fetchPalettes implementation makes changes to the user stratum - this means that when a share-link is generated, the palettes will be included.

Can you please move the fetchPalettes logic into a new LoadableStratum (you can call it NcMWSGetMetadataStratum) ?

nf-s · 2024-05-03T07:14:57Z

lib/Models/Catalog/Ows/WebMapServiceCatalogItem.ts

+
          runInAction(() => {
            const styles = this.styleSelectableDimensions.map(
              (style) => style.selectedId || ""
            );
-            styles[layerIndex] = newStyle;
-            this.setTrait(stratumId, "styles", styles.join(","));
+            styles[layerIndex] = newStyle ? newStyle : "";
+
+            if (this.isNcWMS && this.palette) {
+              const currentStyle = newStyle.split("/")[0];
+              const currentPalette = this.palette;
+              const newStyles = currentStyle + "/" + currentPalette;
+              this.setTrait(stratumId, "styles", newStyles);
+              if (this.noPaletteStyles.includes(newStyle)) {
+                this.setTrait(stratumId, "showPalettes", false);
+              } else {
+                this.setTrait(stratumId, "showPalettes", true);
+              }
+            } else {
+              this.setTrait(stratumId, "styles", styles.join(","));
+            }


Same here, I think you should keep styles and palette separate. After creating the NcWMSLoadableStratum, you can change showPalette there without explicitly calling setTrait.

nf-s · 2024-05-03T07:15:55Z

lib/Models/Catalog/Ows/WebMapServiceCatalogItem.ts

+    if (this.palettes.length === 0) {
+      this.fetchPalettes();
+    }


This should be loaded after GetCapabilitiesStratum

nf-s · 2024-05-03T07:18:25Z

lib/Models/Catalog/Ows/WebMapServiceCatalogItem.ts

+          //if (this.stylesArray[0]) {
+          runInAction(() => {
+            this.setTrait(stratumId, "palette", newPalette);
+            const currentStyle = this.stylesArray[0]
+              ? this.stylesArray[0]
+              : "default";
+            const newStyles = currentStyle.split("/")[0] + "/" + newPalette;
+            this.setTrait(stratumId, "styles", newStyles);


I think there are some issues with updating styles like this - you should keep palette and styles separate.

With the current implementation, palette is not used unless the user makes changes - for example - if palette was defined in a catalog JSON file, it wouldn't be used.

You can apply the palette to the styles parameter here instead. You should only apply the palette to the first style - as this maps to the first layers style.

terriajs/lib/Models/Catalog/Ows/WebMapServiceCatalogItem.ts

Line 566 in bacefe0

parameters.styles = this.styles ?? "";

sixlighthouses added 4 commits February 8, 2024 21:18

read list of palettes from Thredds server layer metadata service

e09266a

temp fix for using availablePalettes trait

33e4694

use 2 new traits for Thredds Palettes in WebMapServiceCatalogItemTrai…

ad99869

…ts and add the fetchPalettes async function to get palettes from styles GetMetadata url

Merge remote-tracking branch 'origin/main' into thredds-palettes

f00d68a

sixlighthouses mentioned this pull request Feb 25, 2024

THREDDS 5.x: Add support for palettes returned by THREDDS GetMetadata call #6893

Open

sixlighthouses marked this pull request as ready for review March 11, 2024 23:45

sixlighthouses added 2 commits March 12, 2024 16:38

Merge remote-tracking branch 'origin/main' into thredds-palettes

7e2b20e

merge main and update CHANGES

1db060e

nf-s assigned ljowen Mar 14, 2024

ljowen reviewed Mar 14, 2024

View reviewed changes

sixlighthouses and others added 7 commits March 14, 2024 20:28

Update lib/Models/Catalog/Ows/WebMapServiceCapabilitiesStratum.ts

9eefa33

Co-authored-by: Lawrence Owen <lawrence.owen@csiro.au>

Update lib/Models/Catalog/Ows/WebMapServiceCapabilitiesStratum.ts

1a477f4

Co-authored-by: Lawrence Owen <lawrence.owen@csiro.au>

Update lib/Models/Catalog/Ows/WebMapServiceCapabilitiesStratum.ts

1227d07

Co-authored-by: Lawrence Owen <lawrence.owen@csiro.au>

fix prettier issue

ecabd69

Merge remote-tracking branch 'origin/main' into thredds-palettes

dc1a2b5

remove superfluous anon function

3e23262

Merge remote-tracking branch 'origin/main' into thredds-palettes

e9209ff

ljowen reviewed Mar 19, 2024

View reviewed changes

if no palette URL is in the style abstract resolve and return [] rath…

54b4420

…er than rejecting

ljowen approved these changes Mar 26, 2024

View reviewed changes

Merge remote-tracking branch 'origin/main' into thredds-palettes

1d22d63

nf-s requested changes Mar 28, 2024

View reviewed changes

sixlighthouses added 4 commits April 2, 2024 16:34

remove thredds prefix from traits name, use the uri.clone method to g…

843ab42

…et the metadata url for each layer

Merge remote-tracking branch 'origin/main' into thredds-palettes

086f24b

wip

edc06b2

Merge remote-tracking branch 'origin/main' into thredds-palettes

38c8c9d

sixlighthouses commented Apr 10, 2024

View reviewed changes

sixlighthouses added 5 commits April 10, 2024 12:39

move initial fetchPalettes into forceLoadMetadata

a14d46e

palettes correctly persisting after style change

0fd38b8

wip

9f91a7d

correct the check for undefined default style and remove console.logs

41892d2

run prettier

722db12

sixlighthouses added 2 commits April 18, 2024 09:56

Merge remote-tracking branch 'origin/main' into thredds-palettes

2528d60

add CHANGES entry

bacefe0

nf-s requested changes May 3, 2024

View reviewed changes

sixlighthouses added 10 commits May 7, 2024 12:46

Merge branch 'main' into thredds-palettes

0dfd466

Merge branch 'main' into thredds-palettes

283bda1

wip

f8d87f0

Merge remote-tracking branch 'origin/main' into thredds-palettes

c07fb88

wip

e8fc7b6

wip

2ee6c8f

correct the traits for new loadable stratum NCWMSGetMetaDataStratum

d8d11b6

remove unused imports

2ddd71c

Merge remote-tracking branch 'origin/main' into thredds-palettes

d06645c

wip

545cf4b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Thredds palettes #7059

Thredds palettes #7059

sixlighthouses commented Feb 23, 2024 •

edited

sixlighthouses commented Feb 25, 2024

sppigot commented Mar 8, 2024

ljowen Mar 14, 2024

sixlighthouses Mar 19, 2024

sixlighthouses Mar 19, 2024

ljowen Mar 19, 2024 •

edited

sixlighthouses Mar 20, 2024 •

edited

nf-s left a comment •

edited

nf-s Mar 28, 2024

nf-s Apr 3, 2024

sixlighthouses left a comment

sixlighthouses left a comment •

edited

sixlighthouses commented Apr 15, 2024

nf-s left a comment •

edited

nf-s May 3, 2024

nf-s May 3, 2024

nf-s May 3, 2024 •

edited

nf-s May 3, 2024

nf-s May 3, 2024

Thredds palettes #7059

Are you sure you want to change the base?

Thredds palettes #7059

Conversation

sixlighthouses commented Feb 23, 2024 • edited

What this PR does

Test me

Checklist

sixlighthouses commented Feb 25, 2024

sppigot commented Mar 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ljowen Mar 19, 2024 • edited

Choose a reason for hiding this comment

sixlighthouses Mar 20, 2024 • edited

Choose a reason for hiding this comment

nf-s left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sixlighthouses left a comment

Choose a reason for hiding this comment

sixlighthouses left a comment • edited

Choose a reason for hiding this comment

sixlighthouses commented Apr 15, 2024

nf-s left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nf-s May 3, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sixlighthouses commented Feb 23, 2024 •

edited

ljowen Mar 19, 2024 •

edited

sixlighthouses Mar 20, 2024 •

edited

nf-s left a comment •

edited

sixlighthouses left a comment •

edited

nf-s left a comment •

edited

nf-s May 3, 2024 •

edited