Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Implement
geom_equals
and binary predicates that depend only on it. #926Implement
geom_equals
and binary predicates that depend only on it. #926Changes from 58 commits
67a2a6a
3038253
d7a842b
3301b95
f6688d7
77b6858
c69d2e6
de923c5
67b0df6
b4ea5e5
305f937
707217b
376833a
f0d0c46
441a585
e02a669
6444a4c
29870d4
c2cc722
18631e3
a1ab8a7
023247e
3ddf7c4
82f40ec
b71f34f
5338a73
af5ae69
34b3fbf
7be5a73
230b0b7
e2a3616
04cd58d
c9211f3
a2acb0c
f749ca0
e94293a
3e7dacd
86d30b6
2c2de39
5448dab
5b39f70
c7f1d21
56865de
57a2d46
b5002e2
3e1d4d5
fdc2fe6
32fb2a4
6b0c4c7
5996035
049b15f
4356f24
9dffa70
654c28e
212881e
61a4108
5416e90
96666a5
21f694c
7854069
10c0498
5278b5e
dbcb617
4a5ff80
ce3aa49
d88c33b
3a7e04f
b7d4d17
01789d7
b53fb6c
a9e6563
41ead90
c7e3686
2b857db
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Why do you need to count the number of points in a feature? Isn't it just the size of the feature, which is stored? (Constant time vs. linear time)
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.
In this case code complexity is reduced by using the
point_indices
object that is an argument topostprocess
. Instead of having to handle all types and combinations in each postprocess, the relevant indices for a given row of theGeoSeries
, which vary across geometry types, are computed in thepreprocess
step and then carried through theBinaryPredicate
operations.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.
The size of each feature in the column isn't stored. But can probably be easily computed with
offest[1:] - offset[:-1]
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.
However, you meant to compute "is all points of the feature contained in the polygon". Isn't it just a
logical_or
to PiP result of the same feature, aka a groupbymax
?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.
Since the result of
BOOL8
is actuallyuint8
, if you dosum
and there are more than 256 points in the multipoint, will it overflow?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.
In the preprocess step,
cuspatial/python/cuspatial/cuspatial/core/binpreds/binpreds.py
Lines 172 to 197 in b7d4d17
GeoSeries
of points. At that timepoint_indices
is also recorded. In order to not use point_indices here that logic would have to be reproduced in thepostprocess
as well.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.
What is this doing?
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.
len(a)