-
Notifications
You must be signed in to change notification settings - Fork 97
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
Unsquish after #600 #603
Unsquish after #600 #603
Conversation
Also some minor errors in
|
I guess |
/// | ||
/// Only wkbPoint[X], wkbLineString[X] or wkbCircularString[X] may alter `out_points`. Other geometry types will silently do nothing, see |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rephrased this, since it wasn't exactly correct (MultiPoint has points, for example).
r? @JmsPae (that's a review request, I can't pick you from the reviewers box) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No complaints here.
Upon some reflection, however, it was weird for get_points to append to a vec instead of overwriting it, but that could be addressed elsewhere if you just want to push this PR out. |
I thought so too, but it matches e.g. |
Thank you! I'd prefer if it follows the original lib's behavior and overwrites the vec, but whether or not you want to fix it here or want to keep it is up to you. (I'm scheming for a switch over to |
I think appending makes sense in some cases (maybe you're producing something like WKB or another compact format), so I wouldn't mind keeping it for now. |
Yea, I suppose if we end up 'fixing' it back to overwriting the vec then we can always add another method for appends. |
CHANGES.md
if knowledge of this change could be valuable to users.