Skip to content

fix: Consolidate RasterPaintLayer #1607

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Apr 30, 2025
Merged

Conversation

hanbyul-here
Copy link
Collaborator

@hanbyul-here hanbyul-here commented Apr 10, 2025

Related Ticket: #1545

Description of Changes

  • I need to confirm how we can get wmts tile url from non raster timeseries data.

Validation / Testing

Load all types of datasets (raster, CMR (HLS data - you will need to zoom to minnesota to see the data), WMS (any ESRI data that @Slesa added: https://deploy-preview-1607--veda-ui.netlify.app/exploration?search=arcgis&datasets=%5B%5D&taxonomy=%7B%7D ) - I believe we no longer have a Zarr dataset on production). Ensure they are working as expected. 'Expectation' includes tile loading, zooming to the dataset for raster timeseries data, datetime change, and 'Load into GIS' functionality.

@aboydnw - Can I ask to test if all the layers work as expected? I created a preview on veda config in case this helps: NASA-IMPACT/veda-config#837
@slesaad - Can you check if I am not missing anything from wms layer?

Copy link

netlify bot commented Apr 10, 2025

Deploy Preview for veda-ui ready!

Name Link
🔨 Latest commit f9bc23c
🔍 Latest deploy log https://app.netlify.com/sites/veda-ui/deploys/6810f4bd6ba6be00083ca293
😎 Deploy Preview https://deploy-preview-1607--veda-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@hanbyul-here hanbyul-here force-pushed the 1545-move-to-raster-paint-layer branch from 8794713 to 3f4912c Compare April 10, 2025 07:07
@hanbyul-here hanbyul-here changed the title [do not merge this yet] Consolidate RasterPaintLayer fix; [do not merge this yet] Consolidate RasterPaintLayer Apr 10, 2025
@hanbyul-here hanbyul-here changed the title fix; [do not merge this yet] Consolidate RasterPaintLayer fix: [do not merge this yet] Consolidate RasterPaintLayer Apr 10, 2025
@hanbyul-here hanbyul-here changed the title fix: [do not merge this yet] Consolidate RasterPaintLayer fix: Consolidate RasterPaintLayer Apr 21, 2025
@hanbyul-here hanbyul-here marked this pull request as ready for review April 21, 2025 05:29
@hanbyul-here hanbyul-here requested a review from aboydnw April 21, 2025 11:13
@slesaad
Copy link
Member

slesaad commented Apr 21, 2025

@hanbyul-here i think you've got it all covered!

@aboydnw
Copy link
Contributor

aboydnw commented Apr 21, 2025

@hanbyul-here I went through some of the stories and datasets where we have had bugs recently and noticed a few datasets that I couldn't get to load properly. I think these show up in the LA and Lahaina wildfires stories.

Otherwise I didn't notice other issues

@aboydnw
Copy link
Contributor

aboydnw commented Apr 21, 2025

I confirmed I'm able to see these in production and in the preview for v6.5.0

@hanbyul-here hanbyul-here force-pushed the 1545-move-to-raster-paint-layer branch from 19783f4 to 71f7827 Compare April 23, 2025 05:56
@hanbyul-here
Copy link
Collaborator Author

hanbyul-here commented Apr 23, 2025

thank you for throughly testing @aboydnw 🙇 . I handled the case, and also added a test for titiler parameter formatter ( any test case idea is welcome!)

edit: I just noticed that nlcd land cover class dataset still doesn't load : https://deploy-preview-837--visex.netlify.app/exploration?datasets=%5B%7B%22id%22%3A%22campfire-nlcd%22%2C%22settings%22%3A%7B%22isVisible%22%3Atrue%2C%22opacity%22%3A100%2C%22analysisMetrics%22%3A%5B%7B%22id%22%3A%22mean%22%2C%22label%22%3A%22Average%22%2C%22chartLabel%22%3A%22Average%22%2C%22themeColor%22%3A%22infographicB%22%7D%2C%7B%22id%22%3A%22std%22%2C%22label%22%3A%22St+Deviation%22%2C%22chartLabel%22%3A%22St+Deviation%22%2C%22themeColor%22%3A%22infographicD%22%7D%5D%7D%7D%2C%7B%22id%22%3A%22nlcd-annual-conus%22%2C%22settings%22%3A%7B%22isVisible%22%3Afalse%2C%22opacity%22%3A100%2C%22analysisMetrics%22%3A%5B%7B%22id%22%3A%22mean%22%2C%22label%22%3A%22Average%22%2C%22chartLabel%22%3A%22Average%22%2C%22themeColor%22%3A%22infographicB%22%7D%2C%7B%22id%22%3A%22std%22%2C%22label%22%3A%22St+Deviation%22%2C%22chartLabel%22%3A%22St+Deviation%22%2C%22themeColor%22%3A%22infographicD%22%7D%5D%2C%22colorMap%22%3A%22nlcd%22%7D%7D%5D&taxonomy=%7B%7D&search=&date=2019-05-03T15%3A00%3A00.000Z

edit of edit: wait then I noticed that nlcd landcover class dataset doesn't load on production either: https://earthdata.nasa.gov/dashboard/exploration?datasets=%5B%7B%22id%22%3A%22campfire-nlcd%22%2C%22settings%22%3A%7B%22isVisible%22%3Atrue%2C%22opacity%22%3A100%2C%22analysisMetrics%22%3A%5B%7B%22id%22%3A%22mean%22%2C%22label%22%3A%22Average%22%2C%22chartLabel%22%3A%22Average%22%2C%22themeColor%22%3A%22infographicB%22%7D%2C%7B%22id%22%3A%22std%22%2C%22label%22%3A%22St+Deviation%22%2C%22chartLabel%22%3A%22St+Deviation%22%2C%22themeColor%22%3A%22infographicD%22%7D%5D%7D%7D%2C%7B%22id%22%3A%22nlcd-annual-conus%22%2C%22settings%22%3A%7B%22isVisible%22%3Afalse%2C%22opacity%22%3A100%2C%22analysisMetrics%22%3A%5B%7B%22id%22%3A%22mean%22%2C%22label%22%3A%22Average%22%2C%22chartLabel%22%3A%22Average%22%2C%22themeColor%22%3A%22infographicB%22%7D%2C%7B%22id%22%3A%22std%22%2C%22label%22%3A%22St+Deviation%22%2C%22chartLabel%22%3A%22St+Deviation%22%2C%22themeColor%22%3A%22infographicD%22%7D%5D%2C%22colorMap%22%3A%22nlcd%22%7D%7D%5D&taxonomy=%7B%7D&search=&date=2016-08-29T15%3A00%3A00.000Z

@hanbyul-here
Copy link
Collaborator Author

@AliceR 🤔 I do not know why but I think the change here removed the gap between point layer -> raster layer!

@hanbyul-here hanbyul-here requested a review from AliceR April 23, 2025 05:58
Copy link
Contributor

@aboydnw aboydnw left a comment

Choose a reason for hiding this comment

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

I did another manual test and didn't notice any issues

@hanbyul-here hanbyul-here merged commit df9e614 into main Apr 30, 2025
10 checks passed
@hanbyul-here hanbyul-here deleted the 1545-move-to-raster-paint-layer branch April 30, 2025 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants