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
[WIP] Use GeometryOpsCore for real #223
base: main
Are you sure you want to change the base?
[WIP] Use GeometryOpsCore for real #223
Changes from 24 commits
d9ff60b
f50c82a
324fccb
36bb92c
e40db7d
2009463
611a726
86f3b87
73f486b
1f5613a
b07808f
7ea0676
50cdbef
91ee1ca
739c55b
a41c257
53936f1
60593d6
3841ec8
6177640
b281ee6
a38f74a
2c79028
2f29e24
39b51f9
ae6c92c
f526f54
c70d007
1663f70
f409149
1f8ff94
d0c0806
907fac6
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.
Feels like we should remove the underscores if we are importing these?
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.
Good point. We can probably make these uppercase, but only exported from Core (not GeometryOps proper)?
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.
Yep perfect
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.
Just realised we can use all of these in Rasters.jl too as its all the same there already. So no underscores is good.
Really keen to unify GeometryOps/Rasters around this core for all geometry related things now.
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.
Again the underscores on imported objects...
Check warning on line 149 in src/transformations/segmentize.jl
Codecov / codecov/patch
src/transformations/segmentize.jl#L149
Check warning on line 177 in src/transformations/segmentize.jl
Codecov / codecov/patch
src/transformations/segmentize.jl#L176-L177
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.
Seems like code duplication with the function below, are both needed?
Are these asserts not checked twice when called from the other method?
(Also throwing an error is better than asserts as it's guaranteed to actually run)
Check warning on line 187 in src/transformations/segmentize.jl
Codecov / codecov/patch
src/transformations/segmentize.jl#L187
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 same assert is applied again above
Check warning on line 190 in src/transformations/segmentize.jl
Codecov / codecov/patch
src/transformations/segmentize.jl#L189-L190