-
Notifications
You must be signed in to change notification settings - Fork 132
Multi-feature search support added to CMR api calls #1109
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
base: main
Are you sure you want to change the base?
Changes from all commits
10c9f86
da1025b
bbbbfd8
37f0b25
5739564
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -851,6 +851,26 @@ def point(self, lon: FloatLike, lat: FloatLike) -> Self: | |||||||||||||||||||||
| """ | ||||||||||||||||||||||
| return super().point(lon, lat) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def multipoint(self, lon_lat_pairs: Sequence[PointLike]) -> Self: | ||||||||||||||||||||||
| """Filter by granules that include multiple geographic points. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Parameters: | ||||||||||||||||||||||
| lon_lat_pairs: sequence of (lon, lat) tuples | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Returns: | ||||||||||||||||||||||
| self | ||||||||||||||||||||||
| """ | ||||||||||||||||||||||
| points = [] | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| for x, y in lon_lat_pairs: | ||||||||||||||||||||||
| self.point(x, y) | ||||||||||||||||||||||
| points.append(self.params.pop('point')[0]) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| self.params['point'] = points | ||||||||||||||||||||||
|
Comment on lines
+863
to
+869
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be unnecessary because the Therefore, this should be all that's required:
Suggested change
|
||||||||||||||||||||||
| self.options['point'] = {'or': True} | ||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer not to assume the caller wants the
Suggested change
|
||||||||||||||||||||||
| return self | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| @override | ||||||||||||||||||||||
| def polygon(self, coordinates: Sequence[PointLike]) -> Self: | ||||||||||||||||||||||
| """Filter by granules that overlap a polygonal area. Must be used in combination | ||||||||||||||||||||||
|
|
@@ -869,6 +889,25 @@ def polygon(self, coordinates: Sequence[PointLike]) -> Self: | |||||||||||||||||||||
| """ | ||||||||||||||||||||||
| return super().polygon(coordinates) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def multipolygon(self, multi_coordinates: Sequence[Sequence[PointLike]]) -> Self: | ||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add an
Suggested change
|
||||||||||||||||||||||
| """Filter by granules that overlap any polygonal area from an input list. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Parameters: | ||||||||||||||||||||||
| multi_coordinates: list of lists of (lon, lat) tuples | ||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Returns: | ||||||||||||||||||||||
| self | ||||||||||||||||||||||
| """ | ||||||||||||||||||||||
| polygons = [] | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| for polygon in multi_coordinates: | ||||||||||||||||||||||
| self.polygon(polygon) | ||||||||||||||||||||||
| polygons.append(self.params.pop('polygon')) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| self.params['polygon'] = polygons | ||||||||||||||||||||||
|
Comment on lines
+901
to
+907
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
| self.options['polygon'] = {'or': True} | ||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
| return self | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| @override | ||||||||||||||||||||||
| def bounding_box( | ||||||||||||||||||||||
| self, | ||||||||||||||||||||||
|
|
@@ -895,6 +934,25 @@ def bounding_box( | |||||||||||||||||||||
| return super().bounding_box( | ||||||||||||||||||||||
| lower_left_lon, lower_left_lat, upper_right_lon, upper_right_lat | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def multi_bounding_box(self, boxes: Sequence[Tuple[FloatLike, FloatLike, FloatLike, FloatLike]]) -> Self: | ||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
| """Filter by granules that overlap any bounding box from an input list. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Parameters: | ||||||||||||||||||||||
| boxes: list of tuples of (lower_left_lon, lower_left_lat, upper_right_lon, upper_right_lat) | ||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Returns: | ||||||||||||||||||||||
| self | ||||||||||||||||||||||
| """ | ||||||||||||||||||||||
| bboxes = [] | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| for box in boxes: | ||||||||||||||||||||||
| self.bounding_box(*box) | ||||||||||||||||||||||
| bboxes.append(self.params.pop('bounding_box')) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| self.params['bounding_box'] = bboxes | ||||||||||||||||||||||
|
Comment on lines
+947
to
+953
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
| self.options['bounding_box'] = {'or': True} | ||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
| return self | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| @override | ||||||||||||||||||||||
| def line(self, coordinates: Sequence[PointLike]) -> Self: | ||||||||||||||||||||||
|
|
@@ -913,6 +971,44 @@ def line(self, coordinates: Sequence[PointLike]) -> Self: | |||||||||||||||||||||
| pairs, or a coordinate could not be converted to a float. | ||||||||||||||||||||||
| """ | ||||||||||||||||||||||
| return super().line(coordinates) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def multiline(self, multi_coordinates: Sequence[Sequence[PointLike]]) -> Self: | ||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
| """Filter by granules that overlap any series of connected points from an input list. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Parameters: | ||||||||||||||||||||||
| multi_coordinates: a list of lists of (lon, lat) tuples | ||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Returns: | ||||||||||||||||||||||
| self | ||||||||||||||||||||||
| """ | ||||||||||||||||||||||
| lines = [] | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| for line in multi_coordinates: | ||||||||||||||||||||||
| self.line(line) | ||||||||||||||||||||||
| lines.append(self.params.pop('line')) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| self.params['line'] = lines | ||||||||||||||||||||||
|
Comment on lines
+984
to
+990
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
| self.options['line'] = {'or': True} | ||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
| return self | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def multicircle(self, multi_circles: Sequence[Tuple[FloatLike,FloatLike,FloatLike]]) -> Self: | ||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
| """Filter by granules that overlap any circle from an input list. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Parameters: | ||||||||||||||||||||||
| multi_circles: list of tuples of (lon, lat, radius) | ||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Returns: | ||||||||||||||||||||||
| self | ||||||||||||||||||||||
| """ | ||||||||||||||||||||||
| circles = [] | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| for circle in multi_circles: | ||||||||||||||||||||||
| self.circle(*circle) | ||||||||||||||||||||||
| circles.append(self.params.pop('circle')) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| self.params['circle'] = circles | ||||||||||||||||||||||
|
Comment on lines
+1003
to
+1009
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
| self.options['circle'] = {'or': True} | ||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
| return self | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| @override | ||||||||||||||||||||||
| def downloadable(self, downloadable: bool = True) -> Self: | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
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.
Please add an or_ kwarg so the caller can explicitly set the option, instead of assuming it should be set to True (and please update the docstring to include it):
I would prefer
or_to be a kwarg because passing a bool value without a keyword is not intuitive.However, making it a positional arg allows for it to be set like so (whereas this is not possible when
or_is a kwarg):Again, it's not intuitive as to what
Truemeans here, so I'd like some input from others on this point (cc: @betolink, @mfisher87, @jhkennedy).I don't like the "bare" boolean argument, but on the other hand, I don't like that (sub)kwargs cannot be specified via
search_data.In order to pass
or_when it is a kwarg, the caller cannot usesearch_data, and must instead directly construct aDataGranulesinstance, like so:(Alternatively, renaming
or_toany_might make it even more readable -- i.e., any/all is more intuitive than or/and, no?)