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

Upgrade to GEOS 3.10.5 and use GeoJSON Reader/Writer #3

Closed
wants to merge 18 commits into from

Conversation

sanak
Copy link
Contributor

@sanak sanak commented Jul 2, 2023

I remembered that geos >= 3.10.0 supports GeoJSON Reader/Writer, so I updated geos version and use GeoJSON Reader/Writer.

Changes proposed in this pull request:

Current performance test (on my M1 MacBook) is as follows and it is a bit faster than turf. 😄

% npm run performance

> [email protected] performance
> node test/performance.mjs

GEOS buffer method took 145.84933400154114 milliseconds to run 1000 times.
@turf/turf buffer method took 147.58733400702477 milliseconds to run 1000 times.

I guess that keeping reader/writer instances during the loop may be better for further performance.
(This means buffer method just accepts geomPtr and returns result bufferPtr for now, but it causes less usability, so something like Geometry object I/O may be better. (Not sure how Shapely manages it.))

=> Delete (strike-through) these lines from #5 (comment).

@chrispahm
Copy link
Owner

Hi @sanak!

Thanks for the PR! 🚀
Using the latest GEOS version is great, thanks for working on that 👍 😊
Before merging this, I'd like to have a discussion about a preferred way of data transfer: #4

So let's first discuss this in the issue!

@sanak
Copy link
Contributor Author

sanak commented Jul 3, 2023

@chrispahm Thanks for response!
I will add my comments to #4 by tomorrow.

@sanak sanak mentioned this pull request Jul 6, 2023
@sanak sanak marked this pull request as draft July 8, 2023 08:18
@sanak
Copy link
Contributor Author

sanak commented Jul 9, 2023

Close this from #5 (comment) discussion.

@sanak sanak closed this Jul 9, 2023
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