Skip to content

add check for invalid geometries before st_filter#142

Merged
fnattino merged 2 commits into
mainfrom
fix_ivalid_geometry
Apr 4, 2025
Merged

add check for invalid geometries before st_filter#142
fnattino merged 2 commits into
mainfrom
fix_ivalid_geometry

Conversation

@SarahAlidoost
Copy link
Copy Markdown
Member

@SarahAlidoost SarahAlidoost commented Mar 31, 2025

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

Related Issues

Added/updated tests?

We encourage you to keep the code coverage percentage at 75% and above.

  • Yes
  • No, and this is why: please replace this line with details on why tests
    have not been included
  • I need help with writing tests

Added entry in changelog?

For user-facing changes, add a line describing the changes in NEWS.md

  • Yes

@fnattino
Copy link
Copy Markdown
Contributor

@SarahAlidoost just a heads up that the Windows R-CMD-check is broken because of #126, so we currently expect it to fail

@SarahAlidoost SarahAlidoost requested a review from fnattino March 31, 2025 09:33
@SarahAlidoost SarahAlidoost marked this pull request as ready for review March 31, 2025 09:33
Copy link
Copy Markdown
Contributor

@fnattino fnattino left a comment

Choose a reason for hiding this comment

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

Thanks a lot @SarahAlidoost for this! As I tried to explain in #115, I think that even working with GEOS vs s2 would not help as invalid polygons would remain as such.
So I think we really have to use sf::st_make_valid().

Maybe this PR could still be useful as it is good to receive the warning if some of the retrieved geometries are invalid. I would however drop the sf::st_use_s2() and re-add instead a call to sf::st_make_valid()..

@SarahAlidoost
Copy link
Copy Markdown
Member Author

Thanks a lot @SarahAlidoost for this! As I tried to explain in #115, I think that even working with GEOS vs s2 would not help as invalid polygons would remain as such. So I think we really have to use sf::st_make_valid().

Maybe this PR could still be useful as it is good to receive the warning if some of the retrieved geometries are invalid. I would however drop the sf::st_use_s2() and re-add instead a call to sf::st_make_valid()..

thanks for checking this. I applied your suggestion by introducing a util function.

@SarahAlidoost SarahAlidoost requested a review from fnattino March 31, 2025 15:08
Copy link
Copy Markdown
Contributor

@fnattino fnattino left a comment

Choose a reason for hiding this comment

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

Sorry for the late reply - this is good to go, thanks @SarahAlidoost!

@fnattino fnattino merged commit 83ced9c into main Apr 4, 2025
@fnattino fnattino deleted the fix_ivalid_geometry branch April 4, 2025 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use of the S2 back-end results in invalid geometries when rertrieving buildings and water surfaces

2 participants