Feat/use titiler cmr #1131
Feat/use titiler cmr #1131
Conversation
✅ Deploy Preview for veda-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
a43cca1
to
ba1b9d9
Compare
|
Yay~ thanks for the great work. I have some questions about titiler parameters for these datasets! While resolving the conflict, I noticed that We recently started using |
| const tileParams = qs.stringify({ | ||
| url: assetUrl, | ||
| time_slice: date, | ||
| ...(assetUrl && { url: assetUrl }), // Only include `url` if `assetUrl` is truthy (not null or undefined) |
There was a problem hiding this comment.
🤔 I wonder if it is best if the raster-paint-layer doesn't need to care about the specifics of the layer- How about we move this logic to Zarr-time-series?
I picture something like the below in zarr-timeseries layer
const zarrSourceParams = { ...sourceParams, assetUrl};
return <RasterPaintLayer {...props} sourceParams={zarrSourceParams} />;
There was a problem hiding this comment.
yes I had something similar to that at one point actually, so happy to make this change. I had all tileParams in the data-type specific layer, so I'll push that change. I think I prefer tileParams to sourceParams since sourceParams can be kept as the static params (the same for all time steps) where as tileParams can include assetUrl and date, which can change whenever the selected date time changes.
| @@ -29,7 +29,7 @@ export function RasterPaintLayer(props: RasterPaintLayerProps) { | |||
|
|
|||
| const { updateStyle } = useMapStyle(); | |||
| const [minZoom] = zoomExtent ?? [0, 20]; | |||
| const generatorId = `zarr-timeseries-${id}`; | |||
| const generatorId = `raster-timeseries-${id}`; | |||
There was a problem hiding this comment.
a nitpick, but it might make sense to let each parent layer pass the generatorPrefix to keep the original way of signaling which layer is which through generator Id
There was a problem hiding this comment.
yes that makes sense and I made that change here: aa6e9b8. I don't fully understand how the generatorId is used however so I can't really test it (other than it doesn't seem to break anything). Could you advise me on if there is a way to manually test that the generatorId is being set? And how is it being used out of curiosity?
There was a problem hiding this comment.
As far as I understand, generator Id is used to uniquely identify the layers for mapboxgl. (but the entire generator id is also suffixed by layer-id - which should be unique - so it will be only useful if there are layers with the same ID but with different data types) And it might come handy if I face a messy situation ex. I have to check layer IDs on the dashboard? It is there for more of a practice rather than actual use at this point.
|
@hanbyul-here thanks for reviewing! In response to #1131 (comment), you are right that in the dataset MDX files I am passing the rescale value for the rescale parameter as a string, not as an array. That is because the tile API, for titiler-xarray (zarr), titiler-cmr (cmr), and VEDA titiler accept the rescale parameter as a comma-delimited string, not an array of arrays. Looking at the titiler changelog and the [documentation] it looks like the API expects multiple rescale parameters for multiple bands, e.g. So I would expect, in the case that we are using data from the STAC renders extension to set tiling parameters, that we need to breakdown the array of arrays into separate rescale parameters. Can you point me to where the parameters are being set by the renders extension? A valid question though is should my code also be using the renders extension (at least, when it exists and is not set in the MDX file which would override the defaults set in STAC metadata) to set these tiling parameters 🤔 . I think it should so I could open a new issue for that! |
|
Ah, I misunderstood in a way that renders extensions format === titiler inputs. You are right that it was about formatting query aprameters - I opened a pr for it : #1140 Thanks for spotting !! 🙇 re: using render extension - All the datasets are being reconciled with STAC data before reaching layers already! (The reconciled data includes render params - but we are giving priority to the input from configuration file. We fall back to render params only when the configuration for sourceParams is missing - we will gradually deprecate manual configuration ! ) |
|
@hanbyul-here I'm taking a look at #1140 I was confused at first because I didn't notice it was a PR to this branch and includes a bit of history we don't have here. I'm not sure why yet but I'll look into it. |
This 1. resolves the conflict with main branch, 2. clarifies how to format values in an array (we might need to revisit this if we want to support multiple values for rescale, but this is at least the coherent behavior with other layers) 3. edits the rescale value for zarr and cmr dataset for consistency
|
@hanbyul-here ok I believe the reason I was seeing so many commits was because you had merged main into feat/use-titiler-cmr and some conflicts needed to be resolved against my changes. I had to add the |
|
@hanbyul-here do you have any other feedback or is this good to merge? |
## 🎉 Features - [E&A] Implement colormap configurability #1117 - [E&A] Feature flag colormap configurability #1147 - Add font md size as 20px for new Design System #1125 ## 🚀 Improvements - [E&A] Return default value of colormap along with other settings #1128 - Create new raster paint layer module and factor out BaseTimeseriesProps #1105 - Consolidate a place where to check linkProps #1121 #1160 - Expose Data Catalog and update child components to pass in routing/nav for library build #1096 #1159 - Set up eslint rule for no trailing spaces #1146 - Replace cmr-stac with titiler-cmr #1131 ## 🐛 Fixes - Update external link in top navigation target #1145 - Update PageHeader/Nav to not throw #1149 - [E&A] Fix to convert the time to usertzdate #1151 - Use sourceparams as it is when it is there #1148 - Fix compare label on block map #1153 - Discard the previous sourceExclusive value to fix the case when the datasets with different sourceExclusive can be selected together #1161 - Show the name of the selected filter on story hub #1161
Related Ticket: #1056
Description of Changes
This change was actually relatively straightforward. Titiler-cmr handles the business of searching CMR for which assets to tile, which means that logic no longer needs to exist in veda-ui. We can just pass the concept id and other parameters directly to titiler-cmr. The asset discovery logic was happening in the useCmr hook which searched CMR STAC endpoints (endpoints plural because there are different catalogues for different DAACs) for data. This was especially messy given the S3 urls are not in the CMR STAC item representations, so we had to replace an https protocol and host with the S3 protocol and bucket name.
Itemized changes:
Notes
datetimewhich is consistent with titiler-cmr, however this change has not yet been deployed to prod-titiler-xarray.delta-backend.com.Validation / Testing
Loaded and toggled both layers in the deploy preview