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

Add test driver for h3-pg#56 #502

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

isaacbrodsky
Copy link
Collaborator

Adds a test driver for zachasme/h3-pg#56 to the core library for debugging this type of polygon that currently has a very high estimation.

@coveralls
Copy link

coveralls commented Jul 27, 2021

Coverage Status

Coverage remained the same at 98.993% when pulling 0953747 on isaacbrodsky:h3pg-56-test into 4b574f0 on uber:master.

@nrabinowitz
Copy link
Collaborator

Not sure I'm following the goal of this PR - the test behavior seems correct. Is the idea that we'll have some strategy to identify cases like this in the future and allocate less memory?

@isaacbrodsky
Copy link
Collaborator Author

Not sure I'm following the goal of this PR - the test behavior seems correct. Is the idea that we'll have some strategy to identify cases like this in the future and allocate less memory?

Yes, this is to help test those cases in the C library, and also ensure we don't regress to requesting more memory in cases like this.

Aside: We may want to consider partial polyfill approaches that would allow us to provide a stream (or iterator) of results to the caller rather than requiring pre-allocating all the memory.

@isaacbrodsky
Copy link
Collaborator Author

Note also that the Win32 test for this actually fails with a segfault, so this does currently fail in some configurations.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Isaac Brodsky seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

4 participants