Skip to content
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

Update indicator data #56

Merged
merged 4 commits into from
Oct 30, 2023
Merged

Update indicator data #56

merged 4 commits into from
Oct 30, 2023

Conversation

corviday
Copy link
Collaborator

@corviday corviday commented Oct 25, 2023

Updates the portal to use the new indicator (netCDF raster) data including coastal regions and further North along the Skeena and Nass basins.

Also implements a whitelist for the predefined watersheds and basins from the BC Freshwater Atlas and predefined salmon conservation units from DFO. Only regions for which we have data will be visible in the dropdowns.

resolves #51
resolves #40 (incidentally, by switching to an entirely new dataset)
resolves #55

I haven't done a linting pass on this PR because there's some file overlap with the next (partially completed) one, and I fear a linting pass would make a merge very complicated.

demo

@rod-glover
Copy link
Contributor

Good call on the linting. I suggest a separate PR (not needing review probably) for linting, each time the project overall needs it. Minor linting can be incorporated in larger PR's of course.

Copy link
Contributor

@rod-glover rod-glover left a comment

Choose a reason for hiding this comment

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

Great work so far. One coding suggestion.

And a question about either data or logic: In the demo, if I select any basin, the watersheds available are always the same. Basin does not affect that selector. Ditto for CU. Is that intended?

public/basins.yaml Show resolved Hide resolved
public/conservation_units.yaml Show resolved Hide resolved
public/watersheds.yaml Show resolved Hide resolved
src/components/AreaDisplay/AreaDisplay.js Outdated Show resolved Hide resolved
src/components/AreaDisplay/AreaDisplay.js Outdated Show resolved Hide resolved
src/helpers/GeographyHelpers.js Show resolved Hide resolved
@corviday
Copy link
Collaborator Author

And a question about either data or logic: In the demo, if I select any basin, the watersheds available are always the same. Basin does not affect that selector. Ditto for CU. Is that intended?

Oops, no! I will look into that.

@corviday
Copy link
Collaborator Author

corviday commented Oct 26, 2023

Hm, it is working "correctly" for me; selecting different basins shows different watersheds and conservation units. Could you perhaps clear your cache and try again, and if it is still not working, see if there are any error messages in the console you can send me?

@rod-glover
Copy link
Contributor

Cleared cache, now watersheds for each basin are mostly disjoint. Fraser and Skeena basins share at least Babine Lake and Babine River watersheds. But they are adjacent and I suspect the polygons overlap in an unfortunate way and that this is a data problem not a logic problem.

That said, the need to clear cache will have to be eliminated at some point. Users won't know to do that.

@rod-glover
Copy link
Contributor

Some 500 errors occur from time to time in the console (and network, of course) log. I can't quite figure out what is provoking them but suspect that invalid or insufficient arguments are being provided to the endpoints. Which are, so far,

This might not be a thing you need to address immediately.

@corviday
Copy link
Collaborator Author

corviday commented Oct 26, 2023

The third is a known data issue - it's trying to find the path from the Fraser watershed to the sea, but of course the Fraser watershed terminates at the sea; there's no path. That's a database fix I haven't made yet.

The previous two are more surprising, those should be valid requests, and both work for me. Hm. That backend throws 500s unexpectedly sometimes; I have wondered if it is too slow to handle a bunch of requests at once. Not a frontend issue, though.

Copy link
Contributor

@rod-glover rod-glover left a comment

Choose a reason for hiding this comment

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

Great work!

@corviday corviday merged commit e65ae4d into main Oct 30, 2023
@corviday corviday deleted the coast branch October 30, 2023 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants