-
Notifications
You must be signed in to change notification settings - Fork 364
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
Filter GeoTiffRDDs by Geometry #2409
Filter GeoTiffRDDs by Geometry #2409
Conversation
bucket: String, prefix: String, | ||
uriToKey: (URI, I) => K, | ||
options: Options, | ||
geometry: Option[Geometry] = None |
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.
Hm, mb to add some helpful overloads? Though it's smth to discuss, as the best otpion would be to move it into Options
and it's impossible due to compatibility reasons. ):
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.
There also needs to be an apply
with the old signature to avoid removing this method from the API.
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.
Mb in 2.0 we'll move geometry
into Options
but for now it's good. Looks like it can be merged as is after #2402
Btw a thought appeared after the review: mb it makes sense to add some extra overloads to use this filter by geometry functionality more convenient. |
Yes, perhaps we can do that in a subsequent PR (or not, depending on timing). |
4fb92b2
to
bbd21aa
Compare
bbd21aa
to
10ec91f
Compare
bucket: String, prefix: String, | ||
uriToKey: (URI, I) => K, | ||
options: Options, | ||
geometry: Option[Geometry] = None |
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.
There also needs to be an apply
with the old signature to avoid removing this method from the API.
10ec91f
to
c33e89b
Compare
Connects #2328