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

TST: skip multisurface test with arrow to avoid crash #479

Conversation

theroggy
Copy link
Member

@theroggy theroggy commented Sep 28, 2024

I wonder if there is any use in delaying the release for this. It will have to be solved upstream... and pyogrio 0.9 with GEOS 3.13 has the same problem...

So, just skipping the test might be OK for now?

Reference: #478

@brendan-ward
Copy link
Member

Thanks for working on this @theroggy . I'm thinking we need a deeper fix here than skipping the tests, since occasionally users do try to work with nonlinear types and we are trying to make use of the Arrow interface more common.

I'm thinking we might be able to use OGRSetNonLinearGeometriesEnabledFlag when reading using Arrow to have a similar effect as what we are already doing when not using Arrow, and then we sidestep the issue in Shapely. Let me try a PR with that.

@brendan-ward
Copy link
Member

Nevermind, OGRSetNonLinearGeometriesEnabledFlag has no effect when reading from GDAL using Arrow, likely by design.

Copy link
Member

@brendan-ward brendan-ward left a comment

Choose a reason for hiding this comment

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

I'm not seeing any way via a GDAL config option that would let us force geometries to linear types when reading from Arrow, so unfortunately this can't really be fixed until handled upstream in Shapely.

Until then skipping this test keeps us green on CI.

pyogrio/tests/test_geopandas_io.py Show resolved Hide resolved
@brendan-ward brendan-ward merged commit bf02284 into geopandas:main Sep 28, 2024
20 of 21 checks passed
@theroggy theroggy deleted the TST-skip-multisurface-test-with-arrow-to-avoid-crash branch September 28, 2024 16:41
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.

2 participants