-
Notifications
You must be signed in to change notification settings - Fork 5
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
feature abundance tiles [MRXN23-193] #1490
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@aagm thank you. I will need a small walkthrough as I haven't actually been able to get this PR to work as I expected: I think I am simply misunderstanding where I should be looking for this data. I have created test projects, and either used features from shapefiles, or uploaded features from puvspr data (csv), selected some features for a new scenario, and saved the feature specification. Once this is done, however, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @aagm! besides what we discussed together (needing to copy amounts over to puvspr_calculations
for features from puvspr and from legacy, but this is a separate concern, not for this PR), please see my notes inline.
In practice, once you've dealt with the inline notes (which could mean do nothing, or just removing the CORS reflection config, up to you), merging this will be ok from my point of view.
But, importantly, if we keep the current setup that changes how feature_list
is shaped in the response tiles, we need to coordinate with FE for them to parse the new format of these tiles correctly.
pu.id AS scenario_pu_id, | ||
ARRAY_AGG(DISTINCT concat_ws(':',species.feature_id::text,pu.amount)) AS feature_list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure we want to let go of the "plain" feature_list (without amounts)?
Thinking about use cases such as showing "how many features are present, irrespective of their amount, in any given PU", I'd lean on keeping that.
For the specific use case of counts of features per PU in practice that could be even compressed to a plain count, not even to an array of the UUIDs of the features present in the PU, and this could be easily done at query time, or through a generated column that holds the count of items in feature_list
.
tl;dr should we insert
the data above into a new column such as feature_and_amount_list
(this being an array of [uuid, float]
pairs, as a new CREATE TYPE feature_amounts AS (feature_id UUID, amount FLOAT)
), and then keep storing the plain list of feature uuids into the existing feature_list
column, and then if we want/need, add a new column such as feature_count int generated always as (cardinality(feature_list)) stored
?
Ammounts at scenario level, this works for all features.
Overview
_Proposed solution that has been proven as valid:
Files touched to produce the tiles:
api/apps/geoprocessing/src/modules/scenario-planning-units-features-aggregate/scenario-planning-units-features-aggregate-processor.ts
We use the amounts stored in geodb
puvspr_calculations
table that is created by merging the information on spatial features and legacy features/csv uploaded ones.
In the vector layer the ammounts are serve through the featureList property under the
planning-units/tiles/{z}/{x}/{y}.mvt?include=features
in the form of
featureList:"<featureid>:<ammount>;<featureid>:<ammount>;"
We will required to store the amount min/max at features and project levels (in a similar way as we will do for cost surface) and provide an endpoint or provide such information in the feature list call used in the pannel.
On the FE side of things the layer styles needs to incorporate this logic:
Where the interpolation bit will depend on the min/max for each feature
Feature relevant tickets
Checklist before submitting
develop
.deploying to staging/production, please add brief testing instructions
to the deploy checklist (
docs/deployment-checklist.md
)