natcap / urban-online-workflow

This repository hosts the beta implementation of the Urban Online ES Workflow. The project is intended to give urban planners the ability to create and assess scenarios using InVEST Urban models.
1 stars 5 forks source link

store LULC codes/names as static data #41

Closed emlys closed 1 year ago

emlys commented 1 year ago

The frontend expects to get LULC codes and names from the backend (used to populate the paint options menu). I'm not seeing a corresponding endpoint that would return the list of LULCs from the database.

davemfish commented 1 year ago

@emlys for now we can also get them from src/landuseCodes.js. I probably should have updated those phony requests for getLulcCodes when we added that file. In the long run I'm not sure if we will keep that data on the front/backend, but it may stay on the frontend.

And I can update the ParcelTable and ScenarioTable and anything else that was using getLulcCodes, so that doesn't need to be part of your effort on the frontend-backend integration.

dcdenu4 commented 1 year ago

Keep us posted, @phargogh , I'm not sure we have the corresponding table of "name" values to the LULC values on the backend?

phargogh commented 1 year ago

Nope, we don't have that yet. I'll implement something and push it to the integration branch.

phargogh commented 1 year ago

Ok, the worker now has an function with the job name raster_classnames (open to better names!) and a return data structure of:

data = {
    'result': {int lulc class: string class name}
}

As a part of this work, I realized I was using the NLUD dataset instead of NLCD. I've updated the worker to use NLCD, which is I believe what the frontend was using anyways.

@dcdenu4 whenver we have an endpoint for the worker to report the stats back, let me know! I have a placeholder in there of /jobsqueue/raster_classnames.

@davemfish @emlys the backend is currently returning the complete attribute table, which consists of 256 entries even though NLCD only has 16 entries for the continental US. Would you prefer that I exclude the blank ones?

phargogh commented 1 year ago

@dcdenu4 I just updated this in response to our live session today to match the structure the frontend is currently using:

data = {
    int: {
        "name": "Forest",
        "color": "#abc123",
    }
}

changes are live on frontend-backend-integration as of 564c799

dcdenu4 commented 1 year ago

Hey @davemfish, given our conversation with Chris about how in the long run we might handle the new updated LULC it probably makes sense to have this on the backend. I'll go ahead and make an endpoint for it, but let me know if you have any updated thoughts on this!

davemfish commented 1 year ago

Okay @dcdenu4 , that makes sense. It's definitely not an urgent need though since we have the data in a frontend module for now.

dcdenu4 commented 1 year ago

@phargogh, I'm wondering what you'd think about moving this functionality out of the worker and into the server api itself or into an adjacent api_utils like module. This functionality especially feels unnecessary to put through the frontend request -> server api -> worker -> server api -> frontend request model.

This really could be a one time calculation that we save / store too... And as I'm typing this, maybe the server api puts this onto the queue at startup which would also allow the frontend to get this info straight away...

davemfish commented 1 year ago

The frontend uses this lookup in a few different components, so it's really convenient right now to be able to import it in whatever module we want to use it.

The alternatives are definitely less appealing: a. request the data from the server in each component that wants to use it b. request the data once from the server and then share it in memory with each component by passing it around in react props.

We know the data is pretty much static; it only changes when the LULC raster gets an update. So maybe we should just keep storing it in the frontend src?

phargogh commented 1 year ago

@dcdenu4 I agree that it's overkill for us to retrieve this information through the work queue for information that is effectively static. I'm hesitant to overload the API layer though with all the GDAL stuff if we don't need to make that layer do spatial things.

I also agree with you @davemfish about the static nature of this information and that it would be useful to just import it on demand.

What if we had a new JSON file, <repo>/appdata/NLCD_2016.lulcdata.json, that has all of this information? Then everyone could load it, read it, etc. and we can just regenerate it or update it as needed. JSON seems like a good serialization format due to native support in both languages. I figure that we could then create a new JSON file when we switch out the basemap and if it has the same file basename (before the .lulcdata.json) and the same schema there shouldn't be much extra work to load the new file.

Thoughts?

davemfish commented 1 year ago

Sounds great to me @phargogh . Right now we're importing frontend/src/landuseCodes.js.

I just confirmed import hello from '../../appdata/hello.json' works. I thought vite might complain about a src file (as opposed to an asset like png) being in the "public" directory. But it seems to work fine.

dcdenu4 commented 1 year ago

This sounds like a good plan to me. So I think to close this issue we need to:

davemfish commented 1 year ago

I'm going to make this happen now as I am wanting to adjust our color choices for the LULC; currently they are not colorblind safe.

davemfish commented 1 year ago

Done in #92