-
Notifications
You must be signed in to change notification settings - Fork 2
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
Merge least feature count polygons with neighbouring polygons #58
Conversation
for more information, see https://pre-commit.ci
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.
Going to approve this, as we can make the param configurable by the user in a future modular approach 👍
'properties', JSONB_BUILD_OBJECT() | ||
'geometry', ST_ASGEOJSON(t.geom)::jsonb, | ||
'properties', JSONB_BUILD_OBJECT( | ||
'building_count', ( |
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.
💫
small_polygon RECORD; -- set small_polygon and nearest_neighbor as record | ||
nearest_neighbor RECORD; -- in order to use them in the loop | ||
BEGIN | ||
min_buildings := num_buildings * 0.5; |
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.
Have you tried modifying this number a bit to see the result?
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.
0.5 is ok, but as a user I would imagine if they requested 15 buildings per task, but only got 8 they might be a bit disappointed 😅
Does 0.7 change things by a lot?
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.
This discrepancy will obviously scale linearly: say they request 50 buildings average and get 25 minimum.
But personally, I think if users are requesting 50 geoms per area there is something wrong with their methodology! I would imagine 10 buildings per task is more realistic for an individual or team 👍
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.
oh crap i didn't see these comments , yeah i had tried with params such as 0.7 but got so big tasks in result. Yeah we can change this in future based on the practical use case scenarios
Description:
This PR introduces a feature to merge small polygons with their neighboring larger polygons based on shared boundary lengths. The goal is to ensure that polygons below a certain area threshold or containing fewer buildings are merged with their best-suited neighbor to avoid fragmentation.
Updates:
A temporary table
leastfeaturepolygons
is created to store polygons smaller than the minimum area obtained from standard deviation or those with fewer than a dynamically calculated number of buildings which is half the number provided my user.A loop is implemented to iterate over all small polygons, finding the neighboring polygon that shares the longest boundary using
ST_Length(ST_Intersection())
.After identifying the best neighbor, the small polygon is merged into the larger one using
ST_Union()
.Controlled Deletion:
Small polygons are deleted from the
taskpolygons
table only after a successful merge, ensuring no polygon is prematurely removed and repeated or overlapped.The neighboring polygon is selected based on the maximum shared boundary, ensuring accurate merges.
Added building count in the properties of geojson
Response:
Before:
num_buildings = 100
After:
num_buildings = 100
Issue
Further splitting if the task polygons contains more features (buildings) than the threshold is yet to be done.