Test polygons are valid after constructing them #156
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mx-moth
commented
Sep 5, 2024
mx-moth
force-pushed
the
test-valid-polygons
branch
from
8a2781d to
d186913
Compare
`Convention.polygons` is now defined on the base `Convention` class. It calls a new abstract method `Convention._make_polygons()`. This allows us to do some post processing of the polygons as part of generating them. Invalid polygons generate a warning and are dropped as part of generation. Non-simple polygons generate a warning but are kept. This means the triangulation code doesn't need to perform this step. It doesn't save any time overall, as the polygons still need to be tested either way.
mx-moth
force-pushed
the
test-valid-polygons
branch
from
d186913 to
0bcc612
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This came about after #154 which was generating invalid polygons which were not picked up. This PR introduces a post-processing step on the
Convention.polygonsproperty which checks the polygons to ensure they are valid polygons. Invalid polygons will spoil other operations so it is good to be certain.This change effectively requires any implementing conventions to rename their
polygonproperty to a_make_polygons()method. The baseConventionclass now includes apolygonproperty which calls this new method and post-processes the results.A new 'hotfix' concept is being trialled here which can adapt a Convention with apolygonproperty and without a_make_polygons()method to the new interface. This is novel but honestly there is only one plugin for emsarray so possibly this is unnecessary extra complications. Still, it is a problem I had been mulling over for some time and I am happy I found a workable solution if only for my own education!Checking for polygon validity will slow things down, but not by a huge amount. The additional safety is worth it. If this safety check becomes burdensome in the future we could consider adding a 'strict' / 'non-strict' mode which enables / disables these sorts checks. In 'non-strict' mode loading datasets would be faster but any errors in the geometry would not be picked up, potentially giving invalid results or otherwise causing errors.