Imageomics / Andromeda

A website that enables users to explore high-dimensional image data
http://andromeda.imageomics.org/
MIT License
2 stars 1 forks source link

Use LatLonCover for landcover data #65

Closed johnbradley closed 9 months ago

johnbradley commented 9 months ago

Replaces hard coded CSV specific to QUEST 2023 with logic to lookup each lat/lon coordinate and provide specific details for each location via LatLonCover. LatLonCover uses the CropScape API to lookup land cover data.

Fetching land cover data takes about 5 seconds per observation. Since this process takes so long the process for retrieving an iNaturalist observation CSV. Only the observations necessary for display are initially retrieved. Once the display observations are retrieved we start building the observation CSV in the background.

Occasionally the CropScape API failed with a read timeout error, requiring some retry logic to provide a reliable API endpoint.

johnbradley commented 9 months ago

I created this as a draft PR since we may be demoing the app Monday.

egrace479 commented 9 months ago

Love the adjustment for the spinner on the Download CSV button! Super effective.

A couple things I noticed testing:

  1. An odd thing (which really isn't terribly important) is that the preview rows show the land cover values in early columns, but when you download the spreadsheet they're way at the end (I tested with also adding RGB).
  2. Actual problem: when I go to upload a spreadsheet that has the satellite RGB values, those columns aren't showing up. sat_url shows up as an image_url option, but the RGB columns don't show up (so if it's just those, there isn't even the option to visualize).
    • I believe this was set in parseCSV.ts and related files that weren't changed...

Screenshot 2023-10-17 at 11 21 05 AM

johnbradley commented 9 months ago

Actual problem: when I go to upload a spreadsheet that has the satellite RGB values, those columns aren't showing up. sat_url shows up as an image_url option, but the RGB columns don't show up (so if it's just those, there isn't even the option to visualize). I believe this was set in parseCSV.ts and related files that weren't changed...

@egrace479 Could this be because those rgb columns all have the same value? There is logic that filtered out columns that don't have more than one value. At some point we discussed changing the UI to let the user know about ignored columns.

egrace479 commented 9 months ago

Actual problem: when I go to upload a spreadsheet that has the satellite RGB values, those columns aren't showing up. sat_url shows up as an image_url option, but the RGB columns don't show up (so if it's just those, there isn't even the option to visualize). I believe this was set in parseCSV.ts and related files that weren't changed...

@egrace479 Could this be because those rgb columns all have the same value? There is logic that filtered out columns that don't have more than one value. At some point we discussed changing the UI to let the user know about ignored columns.

It seems there are only two distinct values per column. But I'm still getting the same problem with this other CSV that has more variation in those columns (I did a fetch for dan_equids). Screenshot 2023-10-17 at 11 38 29 AM

johnbradley commented 9 months ago

It looks like dan_equids now has a observation that doesn't have lat/lon. The numeric columns are all empty for row p36. So javascript throws out the columns because they are not all numeric values.

egrace479 commented 9 months ago

It looks like dan_equids now has a observation that doesn't have lat/lon. The numeric columns are all empty for row p36. So javascript throws out the columns because they are not all numeric values.

So that's why lat/lon columns also aren't showing up for auxiliary columns...We're still getting the land cover because it fills in 0 for all entries that don't have lat/lon values too.

Seems rather problematic, especially since there is no warning thrown.

egrace479 commented 9 months ago

Also, did a sanity check and everything does seem to function just fine when I remove the row with the missing values.

Same for the llhouse2 with only two unique values when I remove the three rows missing lat/lon.

Screenshot 2023-10-17 at 12 05 58 PM

johnbradley commented 9 months ago

I created issue #68 to better explain why certain columns have been excluded from the visualization.

johnbradley commented 9 months ago

For item 1 above:

An odd thing (which really isn't terribly important) is that the preview rows show the land cover values in early columns, but when you download the spreadsheet they're way at the end (I tested with also adding RGB).

This looks to also be a pre-existing problem I can reproduce this on the production Andromeda instance. The screen displays extra columns based on the order of keys in the first object (which seem to be alphabetic).

I created #69 to address this problem.

thompsonmj commented 9 months ago

I'm just noticing that "small" and "big"/"large" aren't defined in the app or in LatLonCover repo README, which is linked to in the Fetch image data tab notes.

Maybe simply adding a link to the outputDataDictionary.txt file in the notes is sufficient?

egrace479 commented 9 months ago

I'm just noticing that "small" and "big"/"large" aren't defined in the app or in LatLonCover repo README, which is linked to in the Fetch image data tab notes.

Maybe simply adding a link to the outputDataDictionary.txt file in the notes is sufficient?

There is a hover-text explaining each on the Fetch Data Tab. It's part of the latlonCover package (and there is a link to the data dictionary explaining it in that README), so I don't know that it's really necessary. We don't have much documentation for the fetch data portion, which should maybe be fleshed out a bit more on the main README along with more explanation of Andromeda, the UI and all that is in this repo. Happy to create a new branch to update our documentation after this PR is addressed.

thompsonmj commented 9 months ago

I only see this for hover-text: image

But yes, this could go into a separate documentation effort 👍

egrace479 commented 9 months ago

I only see this for hover-text: ...

But yes, this could go into a separate documentation effort 👍

I could've sworn the "big" and "small" had hovertext indicating the size. Should come from this no? @johnbradley wasn't that in the original PR?

johnbradley commented 9 months ago

The words small and big have tooltips: Screenshot 2023-10-19 at 8 06 48 AM

johnbradley commented 9 months ago

Here is the html for big and small: https://github.com/Imageomics/Andromeda/blob/c8b8d9b4b0f9da6ae33169947937e0f908a09e5a/andromeda-ui/components/LatLonCoverNote.tsx#L11-L13

Perhaps the tooltips are too subtle.

egrace479 commented 9 months ago

We could consider changing the color of the font or otherwise indicating they're special beyond the italics, but I think everything is functional and works well as currently set up in this PR to proceed. The few areas we think could be improved are documented and are currently behaving in that manner, so ✅🚀

johnbradley commented 9 months ago

@thompsonmj @egrace479 How does this look?

Screenshot 2023-10-19 at 10 49 15 AM
thompsonmj commented 9 months ago

I'd vote to have the dimensions in parentheses right behind those bold terms rather than as a tooltip.

johnbradley commented 9 months ago

Screenshot 2023-10-19 at 11 22 38 AM