From f83ad95341468966473b2a16fd5153cbbdb2f63f Mon Sep 17 00:00:00 2001 From: panda2901 <179280955+angel-cm-dev@users.noreply.github.com> Date: Fri, 3 Apr 2026 23:06:45 -0500 Subject: [PATCH 1/2] feat(queries): add user warnings for flipped coordinates in bounding_box and polygon --- cmr/queries.py | 263 +++++++++++++++++++++-------------------- tests/test_issue_66.py | 31 +++++ 2 files changed, 163 insertions(+), 131 deletions(-) create mode 100644 tests/test_issue_66.py diff --git a/cmr/queries.py b/cmr/queries.py index cdf9f2e..2c3706f 100644 --- a/cmr/queries.py +++ b/cmr/queries.py @@ -8,6 +8,7 @@ from inspect import getmembers, ismethod from re import search from typing import Iterator +import warnings from typing_extensions import ( Any, @@ -22,7 +23,8 @@ Tuple, TypeAlias, Union, - override, deprecated, + override, + deprecated, ) from urllib.parse import quote @@ -48,8 +50,16 @@ class Query: _route = "" _format = "json" _valid_formats_regex = [ - "json", "xml", "echo10", "iso", "iso19115", - "csv", "atom", "kml", "native", "stac", + "json", + "xml", + "echo10", + "iso", + "iso19115", + "csv", + "atom", + "kml", + "native", + "stac", ] def __init__(self, route: str, mode: str = CMR_OPS): @@ -60,7 +70,9 @@ def __init__(self, route: str, mode: str = CMR_OPS): self.concept_id_chars: Set[str] = set() self.headers: MutableMapping[str, str] = {} - @deprecated("Use the 'results' method instead, but note that it produces an iterator.") + @deprecated( + "Use the 'results' method instead, but note that it produces an iterator." + ) def get(self, limit: int = 2000) -> Sequence[Any]: """ Get all results up to some limit, even if spanning multiple pages. @@ -118,7 +130,9 @@ def hits(self) -> int: return int(response.headers["CMR-Hits"]) - @deprecated("Use the 'results' method instead, but note that it produces an iterator.") + @deprecated( + "Use the 'results' method instead, but note that it produces an iterator." + ) def get_all(self) -> Sequence[Any]: """ Returns all of the results for the query. This will call hits() first to determine how many @@ -127,7 +141,7 @@ def get_all(self) -> Sequence[Any]: :returns: query results as a list """ - + return list(self.get(self.hits())) def results(self, page_size: int = 2000) -> Iterator[Any]: @@ -237,13 +251,16 @@ def _build_url(self) -> str: # last chance validation for parameters if not self._valid_state(): - raise RuntimeError(("Spatial parameters must be accompanied by a collection " - "filter (ex: short_name or entry_title).")) + raise RuntimeError( + ( + "Spatial parameters must be accompanied by a collection " + "filter (ex: short_name or entry_title)." + ) + ) # encode params formatted_params = [] for key, val in self.params.items(): - # list params require slightly different formatting if isinstance(val, list): for list_val in val: @@ -261,11 +278,13 @@ def _build_url(self) -> str: formatted_options: List[str] = [] for param_key in self.options: for option_key, val in self.options[param_key].items(): - formatted_options.append(f"options[{param_key}][{option_key}]={str(val).lower()}") + formatted_options.append( + f"options[{param_key}][{option_key}]={str(val).lower()}" + ) options_as_string = "&".join(formatted_options) res = f"{self._base_url}.{self._format}?{params_as_string}&{options_as_string}" - return res.rstrip('&') + return res.rstrip("&") def concept_id(self, IDs: Union[str, Sequence[str]]) -> Self: """ @@ -306,7 +325,7 @@ def provider(self, provider: str) -> Self: if not provider: return self - self.params['provider'] = provider + self.params["provider"] = provider return self @abstractmethod @@ -443,14 +462,12 @@ def online_only(self, online_only: bool = True) -> Self: if "downloadable" in self.params: del self.params["downloadable"] - self.params['online_only'] = online_only + self.params["online_only"] = online_only return self def _format_date( - self, - date_from: Optional[DateLike], - date_to: Optional[DateLike] + self, date_from: Optional[DateLike], date_to: Optional[DateLike] ) -> Tuple[str, str]: """ Format dates into expected format for date queries. @@ -493,8 +510,12 @@ def convert_to_string(date: Optional[DateLike], default: datetime) -> str: return date.strftime(iso_8601) - date_from = convert_to_string(date_from, datetime(1, 1, 1, 0, 0, 0, tzinfo=timezone.utc)) - date_to = convert_to_string(date_to, datetime(1, 12, 31, 23, 59, 59, tzinfo=timezone.utc)) + date_from = convert_to_string( + date_from, datetime(1, 1, 1, 0, 0, 0, tzinfo=timezone.utc) + ) + date_to = convert_to_string( + date_to, datetime(1, 12, 31, 23, 59, 59, tzinfo=timezone.utc) + ) # if we have both dates, make sure from isn't later than to if date_from and date_to and date_from > date_to: @@ -529,9 +550,7 @@ def revision_date( self.params["revision_date"].append(f"{date_from},{date_to}") if exclude_boundary: - self.options["revision_date"] = { - "exclude_boundary": True - } + self.options["revision_date"] = {"exclude_boundary": True} return self @@ -562,9 +581,7 @@ def temporal( self.params["temporal"].append(f"{date_from},{date_to}") if exclude_boundary: - self.options["temporal"] = { - "exclude_boundary": True - } + self.options["temporal"] = {"exclude_boundary": True} return self @@ -579,7 +596,7 @@ def short_name(self, short_name: str) -> Self: if not short_name: return self - self.params['short_name'] = short_name + self.params["short_name"] = short_name return self def version(self, version: str) -> Self: @@ -594,7 +611,7 @@ def version(self, version: str) -> Self: if not version: return self - self.params['version'] = version + self.params["version"] = version return self def point(self, lon: FloatLike, lat: FloatLike) -> Self: @@ -630,23 +647,17 @@ def circle(self, lon: FloatLike, lat: FloatLike, dist: FloatLike) -> Self: :param dist: distance in meters around waypoint (lat,lon) :returns: self """ - self.params['circle'] = f"{lon},{lat},{dist}" + self.params["circle"] = f"{lon},{lat},{dist}" return self def polygon(self, coordinates: Sequence[PointLike]) -> Self: """ - Filter by granules that overlap a polygonal area. Must be used in combination with a - collection filtering parameter such as short_name or entry_title. - - :param coordinates: list of (lon, lat) tuples - :returns: self + Filter by granules that overlap a polygonal area. """ - if not coordinates: return self - # make sure we were passed something iterable try: iter(coordinates) except TypeError: @@ -654,28 +665,32 @@ def polygon(self, coordinates: Sequence[PointLike]) -> Self: f"A line must be an iterable of coordinate tuples. Ex: [(90,90), (91, 90), ...]; got {type(coordinates)}." ) from None - # polygon requires at least 4 pairs of coordinates if len(coordinates) < 4: raise ValueError( f"A polygon requires at least 4 pairs of coordinates; got {len(coordinates)}." ) - # convert to floats as_floats = [] + lons = [] for lon, lat in coordinates: - as_floats.extend([float(lon), float(lat)]) + f_lon, f_lat = float(lon), float(lat) + as_floats.extend([f_lon, f_lat]) + lons.append(f_lon) - # last point must match first point to complete polygon if as_floats[0] != as_floats[-2] or as_floats[1] != as_floats[-1]: raise ValueError( f"Coordinates of the last pair must match the first pair: {coordinates[0]} != {coordinates[-1]}" ) - # convert to strings - as_strs = [str(val) for val in as_floats] + if (max(lons) - min(lons)) > 180: + warnings.warn( + "The polygon's longitude span is greater than 180 degrees. " + "Please verify if the coordinates are flipped or intended to cross the antimeridian.", + UserWarning, + ) + as_strs = [str(val) for val in as_floats] self.params["polygon"] = ",".join(as_strs) - return self def bounding_box( @@ -686,35 +701,38 @@ def bounding_box( upper_right_lat: FloatLike, ) -> Self: """ - Filter by granules that overlap a bounding box. Must be used in combination with - a collection filtering parameter such as short_name or entry_title. - - :param lower_left_lon: lower left longitude of the box - :param lower_left_lat: lower left latitude of the box - :param upper_right_lon: upper right longitude of the box - :param upper_right_lat: upper right latitude of the box - :returns: self + Filter by granules that overlap a bounding box. """ + ll_lon = float(lower_left_lon) + ll_lat = float(lower_left_lat) + ur_lon = float(upper_right_lon) + ur_lat = float(upper_right_lat) - self.params["bounding_box"] = ( - f"{float(lower_left_lon)},{float(lower_left_lat)},{float(upper_right_lon)},{float(upper_right_lat)}" - ) + if ll_lon > ur_lon: + warnings.warn( + f"Coordinates appear to be flipped: lower_left_lon ({ll_lon}) is " + f"greater than upper_right_lon ({ur_lon}). This will result in " + "a bounding box that crosses the antimeridian.", + UserWarning, + ) + if ll_lat > ur_lat: + warnings.warn( + f"Coordinates appear to be flipped: lower_left_lat ({ll_lat}) is " + f"greater than upper_right_lat ({ur_lat}). Please verify the bounding box order.", + UserWarning, + ) + + self.params["bounding_box"] = f"{ll_lon},{ll_lat},{ur_lon},{ur_lat}" return self def line(self, coordinates: Sequence[PointLike]) -> Self: """ - Filter by granules that overlap a series of connected points. Must be used in combination - with a collection filtering parameter such as short_name or entry_title. - - :param coordinates: a list of (lon, lat) tuples - :returns: self + Filter by granules that overlap a series of connected points. """ - if not coordinates: return self - # make sure we were passed something iterable try: iter(coordinates) except TypeError: @@ -722,56 +740,37 @@ def line(self, coordinates: Sequence[PointLike]) -> Self: f"A line must be an iterable of coordinate tuples. Ex: [(90,90), (91, 90), ...]; got {type(coordinates)}." ) from None - # need at least 2 pairs of coordinates if len(coordinates) < 2: raise ValueError( f"A line requires at least 2 pairs of coordinates; got {len(coordinates)}." ) - # make sure they're all floats as_floats = [] for lon, lat in coordinates: as_floats.extend([float(lon), float(lat)]) - # cast back to string for join as_strs = [str(val) for val in as_floats] - self.params["line"] = ",".join(as_strs) - return self def downloadable(self, downloadable: bool = True) -> Self: """ - Only match granules that are available for download. The opposite of this - method is online_only(). - - :param downloadable: True to require granules be downloadable - :returns: self + Only match granules that are available for download. """ - if not isinstance(downloadable, bool): raise TypeError("Downloadable must be of type bool") - # remove the inverse flag so CMR doesn't crash if "online_only" in self.params: del self.params["online_only"] - self.params['downloadable'] = downloadable - + self.params["downloadable"] = downloadable return self def entry_title(self, entry_title: str) -> Self: """ Filter by the collection entry title. - - :param entry_title: Entry title of the collection - :returns: self """ - - entry_title = quote(entry_title) - - self.params['entry_title'] = entry_title - + self.params["entry_title"] = quote(entry_title) return self def platform(self, platform: str) -> Self: @@ -785,7 +784,7 @@ def platform(self, platform: str) -> Self: if not platform: raise ValueError("Please provide a value for platform") - self.params['platform'] = platform + self.params["platform"] = platform return self @@ -811,9 +810,9 @@ def orbit_number( """ if orbit2: - self.params['orbit_number'] = quote(f'{str(orbit1)},{str(orbit2)}') + self.params["orbit_number"] = quote(f"{str(orbit1)},{str(orbit2)}") else: - self.params['orbit_number'] = orbit1 + self.params["orbit_number"] = orbit1 return self @@ -867,7 +866,7 @@ def cloud_cover(self, min_cover: FloatLike = 0, max_cover: FloatLike = 100) -> S "Please ensure min_cover and max_cover are both convertible to floats" ) from None - self.params['cloud_cover'] = f"{min_cover},{max_cover}" + self.params["cloud_cover"] = f"{min_cover},{max_cover}" return self def instrument(self, instrument: str) -> Self: @@ -881,7 +880,7 @@ def instrument(self, instrument: str) -> Self: if not instrument: raise ValueError("Please provide a value for instrument") - self.params['instrument'] = instrument + self.params["instrument"] = instrument return self def sort_key(self, sort_key: str) -> Self: @@ -898,39 +897,39 @@ def sort_key(self, sort_key: str) -> Self: """ valid_sort_keys = { - 'campaign', - 'entry_title', - 'dataset_id', - 'data_size', - 'end_date', - 'granule_ur', - 'producer_granule_id', - 'project', - 'provider', - 'readable_granule_name', - 'short_name', - 'start_date', - 'version', - 'platform', - 'instrument', - 'sensor', - 'day_night_flag', - 'online_only', - 'browsable', - 'browse_only', - 'cloud_cover', - 'revision_date', + "campaign", + "entry_title", + "dataset_id", + "data_size", + "end_date", + "granule_ur", + "producer_granule_id", + "project", + "provider", + "readable_granule_name", + "short_name", + "start_date", + "version", + "platform", + "instrument", + "sensor", + "day_night_flag", + "online_only", + "browsable", + "browse_only", + "cloud_cover", + "revision_date", } # also covers if empty string and allows for '-' prefix (for descending order) - if not isinstance(sort_key, str) or sort_key.lstrip('-') not in valid_sort_keys: + if not isinstance(sort_key, str) or sort_key.lstrip("-") not in valid_sort_keys: raise ValueError( "Please provide a valid sort key for granules query. See" " https://cmr.earthdata.nasa.gov/search/site/docs/search/api.html#sorting-granule-results" " for valid sort keys." ) - self.params['sort_key'] = sort_key + self.params["sort_key"] = sort_key return self def granule_ur(self, granule_ur: str) -> Self: @@ -945,7 +944,7 @@ def granule_ur(self, granule_ur: str) -> Self: if not granule_ur: raise ValueError("Please provide a value for platform") - self.params['granule_ur'] = granule_ur + self.params["granule_ur"] = granule_ur return self def readable_granule_name( @@ -1016,9 +1015,9 @@ class CollectionQuery(GranuleCollectionBaseQuery): def __init__(self, mode: str = CMR_OPS): Query.__init__(self, "collections", mode) self.concept_id_chars = {"C"} - self._valid_formats_regex.extend([ - "dif", "dif10", "opendata", "umm_json", "umm_json_v[0-9]_[0-9]" - ]) + self._valid_formats_regex.extend( + ["dif", "dif10", "opendata", "umm_json", "umm_json_v[0-9]_[0-9]"] + ) def archive_center(self, center: str) -> Self: """ @@ -1029,7 +1028,7 @@ def archive_center(self, center: str) -> Self: """ if center: - self.params['archive_center'] = center + self.params["archive_center"] = center return self @@ -1044,7 +1043,7 @@ def keyword(self, text: str) -> Self: """ if text: - self.params['keyword'] = text + self.params["keyword"] = text return self @@ -1079,7 +1078,9 @@ def tool_concept_id(self, IDs: Union[str, Sequence[str]]) -> Self: # verify we provided with tool concept IDs for ID in IDs: if ID.strip()[0] != "T": - raise ValueError(f"Only tool concept ID's can be provided (begin with 'T'): {ID}") + raise ValueError( + f"Only tool concept ID's can be provided (begin with 'T'): {ID}" + ) self.params["tool_concept_id"] = IDs @@ -1174,14 +1175,13 @@ def get(self, limit: int = 2000) -> Sequence[Any]: results: List[Any] = [] page = 1 while len(results) < limit: - response = requests.get( url, params={"page_size": page_size, "page_num": page} ) response.raise_for_status() if self._format == "json": - latest = response.json()['items'] + latest = response.json()["items"] else: latest = [response.text] @@ -1219,7 +1219,7 @@ def name(self, name: str) -> Self: if not name: return self - self.params['name'] = name + self.params["name"] = name return self @@ -1231,9 +1231,9 @@ class ToolQuery(ToolServiceVariableBaseQuery): def __init__(self, mode: str = CMR_OPS): Query.__init__(self, "tools", mode) self.concept_id_chars = {"T"} - self._valid_formats_regex.extend([ - "dif", "dif10", "opendata", "umm_json", "umm_json_v[0-9]_[0-9]" - ]) + self._valid_formats_regex.extend( + ["dif", "dif10", "opendata", "umm_json", "umm_json_v[0-9]_[0-9]"] + ) @override def _valid_state(self) -> bool: @@ -1248,9 +1248,9 @@ class ServiceQuery(ToolServiceVariableBaseQuery): def __init__(self, mode: str = CMR_OPS): Query.__init__(self, "services", mode) self.concept_id_chars = {"S"} - self._valid_formats_regex.extend([ - "dif", "dif10", "opendata", "umm_json", "umm_json_v[0-9]_[0-9]" - ]) + self._valid_formats_regex.extend( + ["dif", "dif10", "opendata", "umm_json", "umm_json_v[0-9]_[0-9]"] + ) @override def _valid_state(self) -> bool: @@ -1258,13 +1258,12 @@ def _valid_state(self) -> bool: class VariableQuery(ToolServiceVariableBaseQuery): - def __init__(self, mode: str = CMR_OPS): Query.__init__(self, "variables", mode) self.concept_id_chars = {"V"} - self._valid_formats_regex.extend([ - "dif", "dif10", "opendata", "umm_json", "umm_json_v[0-9]_[0-9]" - ]) + self._valid_formats_regex.extend( + ["dif", "dif10", "opendata", "umm_json", "umm_json_v[0-9]_[0-9]"] + ) def instance_format(self, format: Union[str, Sequence[str]]) -> Self: """ @@ -1278,7 +1277,9 @@ def instance_format(self, format: Union[str, Sequence[str]]) -> Self: if format: # Assume we have non-empty string or sequence of strings (list, tuple, etc.) - self.params['instance_format'] = [format] if isinstance(format, str) else format + self.params["instance_format"] = ( + [format] if isinstance(format, str) else format + ) return self diff --git a/tests/test_issue_66.py b/tests/test_issue_66.py new file mode 100644 index 0000000..0ef5130 --- /dev/null +++ b/tests/test_issue_66.py @@ -0,0 +1,31 @@ +import unittest +import warnings +from cmr import queries + + +class TestIssue66(unittest.TestCase): + def test_wkt_coordinate_order_warning(self): + # 1. Usamos coordenadas que cruzan casi todo el mapa (Span > 180) + # De 170 a -170 hay una diferencia de 340 grados. + flipped_coords = [(170, 10), (-170, 10), (-170, -10), (170, -10), (170, 10)] + + query = queries.GranuleQuery() + + # 2. Ahora sí debería dispararse el UserWarning + with self.assertWarns(UserWarning) as cm: + query.polygon(flipped_coords) + + self.assertIn("longitude span is greater than 180 degrees", str(cm.warning)) + + def test_bounding_box_order_warning(self): + # Este ya pasaba, lo mantenemos igual + query = queries.GranuleQuery() + + with self.assertWarns(UserWarning) as cm: + query.bounding_box(10, 0, -10, 5) + + self.assertIn("crosses the antimeridian", str(cm.warning)) + + +if __name__ == "__main__": + unittest.main() From 78ff562a545297cc8c1b78b4dfa30bad3e48c3de Mon Sep 17 00:00:00 2001 From: panda2901 <179280955+angel-cm-dev@users.noreply.github.com> Date: Mon, 6 Apr 2026 20:19:46 -0500 Subject: [PATCH 2/2] refactor: address reviewer feedback - translate messages and migrate tests to test_granule.py --- cmr/queries.py | 240 +++++++++++++++++++++++------------------ tests/test_granule.py | 146 ++++++++++++++++++------- tests/test_issue_66.py | 31 ------ 3 files changed, 241 insertions(+), 176 deletions(-) delete mode 100644 tests/test_issue_66.py diff --git a/cmr/queries.py b/cmr/queries.py index 2c3706f..66ab39c 100644 --- a/cmr/queries.py +++ b/cmr/queries.py @@ -8,7 +8,6 @@ from inspect import getmembers, ismethod from re import search from typing import Iterator -import warnings from typing_extensions import ( Any, @@ -23,12 +22,12 @@ Tuple, TypeAlias, Union, - override, - deprecated, + override, deprecated, ) from urllib.parse import quote import requests +import warnings from dateutil.parser import parse as dateutil_parse CMR_OPS = "https://cmr.earthdata.nasa.gov/search/" @@ -50,16 +49,8 @@ class Query: _route = "" _format = "json" _valid_formats_regex = [ - "json", - "xml", - "echo10", - "iso", - "iso19115", - "csv", - "atom", - "kml", - "native", - "stac", + "json", "xml", "echo10", "iso", "iso19115", + "csv", "atom", "kml", "native", "stac", ] def __init__(self, route: str, mode: str = CMR_OPS): @@ -70,9 +61,7 @@ def __init__(self, route: str, mode: str = CMR_OPS): self.concept_id_chars: Set[str] = set() self.headers: MutableMapping[str, str] = {} - @deprecated( - "Use the 'results' method instead, but note that it produces an iterator." - ) + @deprecated("Use the 'results' method instead, but note that it produces an iterator.") def get(self, limit: int = 2000) -> Sequence[Any]: """ Get all results up to some limit, even if spanning multiple pages. @@ -130,9 +119,7 @@ def hits(self) -> int: return int(response.headers["CMR-Hits"]) - @deprecated( - "Use the 'results' method instead, but note that it produces an iterator." - ) + @deprecated("Use the 'results' method instead, but note that it produces an iterator.") def get_all(self) -> Sequence[Any]: """ Returns all of the results for the query. This will call hits() first to determine how many @@ -141,7 +128,7 @@ def get_all(self) -> Sequence[Any]: :returns: query results as a list """ - + return list(self.get(self.hits())) def results(self, page_size: int = 2000) -> Iterator[Any]: @@ -251,16 +238,13 @@ def _build_url(self) -> str: # last chance validation for parameters if not self._valid_state(): - raise RuntimeError( - ( - "Spatial parameters must be accompanied by a collection " - "filter (ex: short_name or entry_title)." - ) - ) + raise RuntimeError(("Spatial parameters must be accompanied by a collection " + "filter (ex: short_name or entry_title).")) # encode params formatted_params = [] for key, val in self.params.items(): + # list params require slightly different formatting if isinstance(val, list): for list_val in val: @@ -278,13 +262,11 @@ def _build_url(self) -> str: formatted_options: List[str] = [] for param_key in self.options: for option_key, val in self.options[param_key].items(): - formatted_options.append( - f"options[{param_key}][{option_key}]={str(val).lower()}" - ) + formatted_options.append(f"options[{param_key}][{option_key}]={str(val).lower()}") options_as_string = "&".join(formatted_options) res = f"{self._base_url}.{self._format}?{params_as_string}&{options_as_string}" - return res.rstrip("&") + return res.rstrip('&') def concept_id(self, IDs: Union[str, Sequence[str]]) -> Self: """ @@ -325,7 +307,7 @@ def provider(self, provider: str) -> Self: if not provider: return self - self.params["provider"] = provider + self.params['provider'] = provider return self @abstractmethod @@ -462,12 +444,14 @@ def online_only(self, online_only: bool = True) -> Self: if "downloadable" in self.params: del self.params["downloadable"] - self.params["online_only"] = online_only + self.params['online_only'] = online_only return self def _format_date( - self, date_from: Optional[DateLike], date_to: Optional[DateLike] + self, + date_from: Optional[DateLike], + date_to: Optional[DateLike] ) -> Tuple[str, str]: """ Format dates into expected format for date queries. @@ -510,12 +494,8 @@ def convert_to_string(date: Optional[DateLike], default: datetime) -> str: return date.strftime(iso_8601) - date_from = convert_to_string( - date_from, datetime(1, 1, 1, 0, 0, 0, tzinfo=timezone.utc) - ) - date_to = convert_to_string( - date_to, datetime(1, 12, 31, 23, 59, 59, tzinfo=timezone.utc) - ) + date_from = convert_to_string(date_from, datetime(1, 1, 1, 0, 0, 0, tzinfo=timezone.utc)) + date_to = convert_to_string(date_to, datetime(1, 12, 31, 23, 59, 59, tzinfo=timezone.utc)) # if we have both dates, make sure from isn't later than to if date_from and date_to and date_from > date_to: @@ -550,7 +530,9 @@ def revision_date( self.params["revision_date"].append(f"{date_from},{date_to}") if exclude_boundary: - self.options["revision_date"] = {"exclude_boundary": True} + self.options["revision_date"] = { + "exclude_boundary": True + } return self @@ -581,7 +563,9 @@ def temporal( self.params["temporal"].append(f"{date_from},{date_to}") if exclude_boundary: - self.options["temporal"] = {"exclude_boundary": True} + self.options["temporal"] = { + "exclude_boundary": True + } return self @@ -596,7 +580,7 @@ def short_name(self, short_name: str) -> Self: if not short_name: return self - self.params["short_name"] = short_name + self.params['short_name'] = short_name return self def version(self, version: str) -> Self: @@ -611,7 +595,7 @@ def version(self, version: str) -> Self: if not version: return self - self.params["version"] = version + self.params['version'] = version return self def point(self, lon: FloatLike, lat: FloatLike) -> Self: @@ -647,17 +631,23 @@ def circle(self, lon: FloatLike, lat: FloatLike, dist: FloatLike) -> Self: :param dist: distance in meters around waypoint (lat,lon) :returns: self """ - self.params["circle"] = f"{lon},{lat},{dist}" + self.params['circle'] = f"{lon},{lat},{dist}" return self def polygon(self, coordinates: Sequence[PointLike]) -> Self: """ - Filter by granules that overlap a polygonal area. + Filter by granules that overlap a polygonal area. Must be used in combination with a + collection filtering parameter such as short_name or entry_title. + + :param coordinates: list of (lon, lat) tuples + :returns: self """ + if not coordinates: return self + # make sure we were passed something iterable try: iter(coordinates) except TypeError: @@ -665,32 +655,39 @@ def polygon(self, coordinates: Sequence[PointLike]) -> Self: f"A line must be an iterable of coordinate tuples. Ex: [(90,90), (91, 90), ...]; got {type(coordinates)}." ) from None + # polygon requires at least 4 pairs of coordinates if len(coordinates) < 4: raise ValueError( f"A polygon requires at least 4 pairs of coordinates; got {len(coordinates)}." ) + # convert to floats as_floats = [] lons = [] for lon, lat in coordinates: f_lon, f_lat = float(lon), float(lat) as_floats.extend([f_lon, f_lat]) lons.append(f_lon) - + + # last point must match first point to complete polygon if as_floats[0] != as_floats[-2] or as_floats[1] != as_floats[-1]: raise ValueError( f"Coordinates of the last pair must match the first pair: {coordinates[0]} != {coordinates[-1]}" ) - + + # Check for longitude span and trigger warning if it exceeds 180 degrees if (max(lons) - min(lons)) > 180: warnings.warn( "The polygon's longitude span is greater than 180 degrees. " "Please verify if the coordinates are flipped or intended to cross the antimeridian.", UserWarning, ) - + + # convert to strings as_strs = [str(val) for val in as_floats] + self.params["polygon"] = ",".join(as_strs) + return self def bounding_box( @@ -701,8 +698,16 @@ def bounding_box( upper_right_lat: FloatLike, ) -> Self: """ - Filter by granules that overlap a bounding box. + Filter by granules that overlap a bounding box. Must be used in combination with + a collection filtering parameter such as short_name or entry_title. + + :param lower_left_lon: lower left longitude of the box + :param lower_left_lat: lower left latitude of the box + :param upper_right_lon: upper right longitude of the box + :param upper_right_lat: upper right latitude of the box + :returns: self """ + ll_lon = float(lower_left_lon) ll_lat = float(lower_left_lat) ur_lon = float(upper_right_lon) @@ -725,14 +730,20 @@ def bounding_box( self.params["bounding_box"] = f"{ll_lon},{ll_lat},{ur_lon},{ur_lat}" return self - + def line(self, coordinates: Sequence[PointLike]) -> Self: """ - Filter by granules that overlap a series of connected points. + Filter by granules that overlap a series of connected points. Must be used in combination + with a collection filtering parameter such as short_name or entry_title. + + :param coordinates: a list of (lon, lat) tuples + :returns: self """ + if not coordinates: return self + # make sure we were passed something iterable try: iter(coordinates) except TypeError: @@ -740,37 +751,56 @@ def line(self, coordinates: Sequence[PointLike]) -> Self: f"A line must be an iterable of coordinate tuples. Ex: [(90,90), (91, 90), ...]; got {type(coordinates)}." ) from None + # need at least 2 pairs of coordinates if len(coordinates) < 2: raise ValueError( f"A line requires at least 2 pairs of coordinates; got {len(coordinates)}." ) + # make sure they're all floats as_floats = [] for lon, lat in coordinates: as_floats.extend([float(lon), float(lat)]) + # cast back to string for join as_strs = [str(val) for val in as_floats] + self.params["line"] = ",".join(as_strs) + return self def downloadable(self, downloadable: bool = True) -> Self: """ - Only match granules that are available for download. + Only match granules that are available for download. The opposite of this + method is online_only(). + + :param downloadable: True to require granules be downloadable + :returns: self """ + if not isinstance(downloadable, bool): raise TypeError("Downloadable must be of type bool") + # remove the inverse flag so CMR doesn't crash if "online_only" in self.params: del self.params["online_only"] - self.params["downloadable"] = downloadable + self.params['downloadable'] = downloadable + return self def entry_title(self, entry_title: str) -> Self: """ Filter by the collection entry title. + + :param entry_title: Entry title of the collection + :returns: self """ - self.params["entry_title"] = quote(entry_title) + + entry_title = quote(entry_title) + + self.params['entry_title'] = entry_title + return self def platform(self, platform: str) -> Self: @@ -784,7 +814,7 @@ def platform(self, platform: str) -> Self: if not platform: raise ValueError("Please provide a value for platform") - self.params["platform"] = platform + self.params['platform'] = platform return self @@ -810,9 +840,9 @@ def orbit_number( """ if orbit2: - self.params["orbit_number"] = quote(f"{str(orbit1)},{str(orbit2)}") + self.params['orbit_number'] = quote(f'{str(orbit1)},{str(orbit2)}') else: - self.params["orbit_number"] = orbit1 + self.params['orbit_number'] = orbit1 return self @@ -866,7 +896,7 @@ def cloud_cover(self, min_cover: FloatLike = 0, max_cover: FloatLike = 100) -> S "Please ensure min_cover and max_cover are both convertible to floats" ) from None - self.params["cloud_cover"] = f"{min_cover},{max_cover}" + self.params['cloud_cover'] = f"{min_cover},{max_cover}" return self def instrument(self, instrument: str) -> Self: @@ -880,7 +910,7 @@ def instrument(self, instrument: str) -> Self: if not instrument: raise ValueError("Please provide a value for instrument") - self.params["instrument"] = instrument + self.params['instrument'] = instrument return self def sort_key(self, sort_key: str) -> Self: @@ -897,39 +927,39 @@ def sort_key(self, sort_key: str) -> Self: """ valid_sort_keys = { - "campaign", - "entry_title", - "dataset_id", - "data_size", - "end_date", - "granule_ur", - "producer_granule_id", - "project", - "provider", - "readable_granule_name", - "short_name", - "start_date", - "version", - "platform", - "instrument", - "sensor", - "day_night_flag", - "online_only", - "browsable", - "browse_only", - "cloud_cover", - "revision_date", + 'campaign', + 'entry_title', + 'dataset_id', + 'data_size', + 'end_date', + 'granule_ur', + 'producer_granule_id', + 'project', + 'provider', + 'readable_granule_name', + 'short_name', + 'start_date', + 'version', + 'platform', + 'instrument', + 'sensor', + 'day_night_flag', + 'online_only', + 'browsable', + 'browse_only', + 'cloud_cover', + 'revision_date', } # also covers if empty string and allows for '-' prefix (for descending order) - if not isinstance(sort_key, str) or sort_key.lstrip("-") not in valid_sort_keys: + if not isinstance(sort_key, str) or sort_key.lstrip('-') not in valid_sort_keys: raise ValueError( "Please provide a valid sort key for granules query. See" " https://cmr.earthdata.nasa.gov/search/site/docs/search/api.html#sorting-granule-results" " for valid sort keys." ) - self.params["sort_key"] = sort_key + self.params['sort_key'] = sort_key return self def granule_ur(self, granule_ur: str) -> Self: @@ -944,7 +974,7 @@ def granule_ur(self, granule_ur: str) -> Self: if not granule_ur: raise ValueError("Please provide a value for platform") - self.params["granule_ur"] = granule_ur + self.params['granule_ur'] = granule_ur return self def readable_granule_name( @@ -1015,9 +1045,9 @@ class CollectionQuery(GranuleCollectionBaseQuery): def __init__(self, mode: str = CMR_OPS): Query.__init__(self, "collections", mode) self.concept_id_chars = {"C"} - self._valid_formats_regex.extend( - ["dif", "dif10", "opendata", "umm_json", "umm_json_v[0-9]_[0-9]"] - ) + self._valid_formats_regex.extend([ + "dif", "dif10", "opendata", "umm_json", "umm_json_v[0-9]_[0-9]" + ]) def archive_center(self, center: str) -> Self: """ @@ -1028,7 +1058,7 @@ def archive_center(self, center: str) -> Self: """ if center: - self.params["archive_center"] = center + self.params['archive_center'] = center return self @@ -1043,7 +1073,7 @@ def keyword(self, text: str) -> Self: """ if text: - self.params["keyword"] = text + self.params['keyword'] = text return self @@ -1078,9 +1108,7 @@ def tool_concept_id(self, IDs: Union[str, Sequence[str]]) -> Self: # verify we provided with tool concept IDs for ID in IDs: if ID.strip()[0] != "T": - raise ValueError( - f"Only tool concept ID's can be provided (begin with 'T'): {ID}" - ) + raise ValueError(f"Only tool concept ID's can be provided (begin with 'T'): {ID}") self.params["tool_concept_id"] = IDs @@ -1175,13 +1203,14 @@ def get(self, limit: int = 2000) -> Sequence[Any]: results: List[Any] = [] page = 1 while len(results) < limit: + response = requests.get( url, params={"page_size": page_size, "page_num": page} ) response.raise_for_status() if self._format == "json": - latest = response.json()["items"] + latest = response.json()['items'] else: latest = [response.text] @@ -1219,7 +1248,7 @@ def name(self, name: str) -> Self: if not name: return self - self.params["name"] = name + self.params['name'] = name return self @@ -1231,9 +1260,9 @@ class ToolQuery(ToolServiceVariableBaseQuery): def __init__(self, mode: str = CMR_OPS): Query.__init__(self, "tools", mode) self.concept_id_chars = {"T"} - self._valid_formats_regex.extend( - ["dif", "dif10", "opendata", "umm_json", "umm_json_v[0-9]_[0-9]"] - ) + self._valid_formats_regex.extend([ + "dif", "dif10", "opendata", "umm_json", "umm_json_v[0-9]_[0-9]" + ]) @override def _valid_state(self) -> bool: @@ -1248,9 +1277,9 @@ class ServiceQuery(ToolServiceVariableBaseQuery): def __init__(self, mode: str = CMR_OPS): Query.__init__(self, "services", mode) self.concept_id_chars = {"S"} - self._valid_formats_regex.extend( - ["dif", "dif10", "opendata", "umm_json", "umm_json_v[0-9]_[0-9]"] - ) + self._valid_formats_regex.extend([ + "dif", "dif10", "opendata", "umm_json", "umm_json_v[0-9]_[0-9]" + ]) @override def _valid_state(self) -> bool: @@ -1258,12 +1287,13 @@ def _valid_state(self) -> bool: class VariableQuery(ToolServiceVariableBaseQuery): + def __init__(self, mode: str = CMR_OPS): Query.__init__(self, "variables", mode) self.concept_id_chars = {"V"} - self._valid_formats_regex.extend( - ["dif", "dif10", "opendata", "umm_json", "umm_json_v[0-9]_[0-9]"] - ) + self._valid_formats_regex.extend([ + "dif", "dif10", "opendata", "umm_json", "umm_json_v[0-9]_[0-9]" + ]) def instance_format(self, format: Union[str, Sequence[str]]) -> Self: """ @@ -1277,9 +1307,7 @@ def instance_format(self, format: Union[str, Sequence[str]]) -> Self: if format: # Assume we have non-empty string or sequence of strings (list, tuple, etc.) - self.params["instance_format"] = ( - [format] if isinstance(format, str) else format - ) + self.params['instance_format'] = [format] if isinstance(format, str) else format return self diff --git a/tests/test_granule.py b/tests/test_granule.py index fbf188f..07f425b 100644 --- a/tests/test_granule.py +++ b/tests/test_granule.py @@ -1,4 +1,5 @@ import inspect +import pytest import os from datetime import datetime, timezone, timedelta import json @@ -30,7 +31,7 @@ class TestGranuleClass(VCRTestCase): # type: ignore sort_key = "sort_key" def _get_vcr_kwargs(self, **kwargs): - kwargs['decode_compressed_response'] = True + kwargs["decode_compressed_response"] = True return kwargs def _get_cassette_library_dir(self): @@ -86,8 +87,12 @@ def test_circle_set(self): def test_revision_date(self): query = GranuleQuery() - granules = query.short_name("SWOT_L2_HR_RiverSP_reach_2.0").revision_date("2024-07-05", "2024-07-05").format( - "umm_json").get_all() + granules = ( + query.short_name("SWOT_L2_HR_RiverSP_reach_2.0") + .revision_date("2024-07-05", "2024-07-05") + .format("umm_json") + .get_all() + ) granule_dict = {} for granule in granules: granule_json = json.loads(granule) @@ -95,12 +100,18 @@ def test_revision_date(self): native_id = item["meta"]["native-id"] granule_dict[native_id] = item - self.assertIn("SWOT_L2_HR_RiverSP_Reach_017_312_AS_20240630T042656_20240630T042706_PIC0_01_swot", - granule_dict.keys()) - self.assertIn("SWOT_L2_HR_RiverSP_Reach_017_310_SI_20240630T023426_20240630T023433_PIC0_01_swot", - granule_dict.keys()) - self.assertIn("SWOT_L2_HR_RiverSP_Reach_017_333_EU_20240630T225156_20240630T225203_PIC0_01_swot", - granule_dict.keys()) + self.assertIn( + "SWOT_L2_HR_RiverSP_Reach_017_312_AS_20240630T042656_20240630T042706_PIC0_01_swot", + granule_dict.keys(), + ) + self.assertIn( + "SWOT_L2_HR_RiverSP_Reach_017_310_SI_20240630T023426_20240630T023433_PIC0_01_swot", + granule_dict.keys(), + ) + self.assertIn( + "SWOT_L2_HR_RiverSP_Reach_017_333_EU_20240630T225156_20240630T225203_PIC0_01_swot", + granule_dict.keys(), + ) def test_temporal_invalid_strings(self): query = GranuleQuery() @@ -129,27 +140,39 @@ def test_temporal_rounding(self): # one whole year query.temporal("2016", "2016") self.assertIn("temporal", query.params) - self.assertEqual(query.params["temporal"][0], "2016-01-01T00:00:00Z,2016-12-31T23:59:59Z") + self.assertEqual( + query.params["temporal"][0], "2016-01-01T00:00:00Z,2016-12-31T23:59:59Z" + ) # one whole month query.temporal("2016-10", "2016-10") - self.assertEqual(query.params["temporal"][1], "2016-10-01T00:00:00Z,2016-10-31T23:59:59Z") + self.assertEqual( + query.params["temporal"][1], "2016-10-01T00:00:00Z,2016-10-31T23:59:59Z" + ) # one whole day, wrong way query.temporal("2016-10-10", datetime(2016, 10, 10)) - self.assertNotEqual(query.params["temporal"][2], "2016-10-10T00:00:00Z,2016-10-10T23:59:59Z") + self.assertNotEqual( + query.params["temporal"][2], "2016-10-10T00:00:00Z,2016-10-10T23:59:59Z" + ) # one whole day, right way query.temporal("2016-10-10", datetime(2016, 10, 10).date()) - self.assertEqual(query.params["temporal"][3], "2016-10-10T00:00:00Z,2016-10-10T23:59:59Z") + self.assertEqual( + query.params["temporal"][3], "2016-10-10T00:00:00Z,2016-10-10T23:59:59Z" + ) def test_temporal_tz_aware(self): query = GranuleQuery() tz = timezone(timedelta(hours=-3)) - query.temporal("2016-10-10T00:02:01-03:00", datetime(2016, 10, 10, 0, 2, 1, tzinfo=tz)) + query.temporal( + "2016-10-10T00:02:01-03:00", datetime(2016, 10, 10, 0, 2, 1, tzinfo=tz) + ) self.assertIn("temporal", query.params) - self.assertEqual(query.params["temporal"][0], "2016-10-10T03:02:01Z,2016-10-10T03:02:01Z") + self.assertEqual( + query.params["temporal"][0], "2016-10-10T03:02:01Z,2016-10-10T03:02:01Z" + ) def test_temporal_set(self): query = GranuleQuery() @@ -157,12 +180,16 @@ def test_temporal_set(self): # both strings query.temporal("2016-10-10T01:02:03Z", "2016-10-12T09:08:07Z") self.assertIn("temporal", query.params) - self.assertEqual(query.params["temporal"][0], "2016-10-10T01:02:03Z,2016-10-12T09:08:07Z") + self.assertEqual( + query.params["temporal"][0], "2016-10-10T01:02:03Z,2016-10-12T09:08:07Z" + ) # string and datetime query.temporal("2016-10-10T01:02:03Z", datetime(2016, 10, 12, 9)) self.assertIn("temporal", query.params) - self.assertEqual(query.params["temporal"][1], "2016-10-10T01:02:03Z,2016-10-12T09:00:00Z") + self.assertEqual( + query.params["temporal"][1], "2016-10-10T01:02:03Z,2016-10-12T09:00:00Z" + ) # string and None query.temporal(datetime(2016, 10, 12, 10, 55, 7), None) @@ -172,12 +199,16 @@ def test_temporal_set(self): # both datetimes query.temporal(datetime(2016, 10, 12, 10, 55, 7), datetime(2016, 10, 12, 11)) self.assertIn("temporal", query.params) - self.assertEqual(query.params["temporal"][3], "2016-10-12T10:55:07Z,2016-10-12T11:00:00Z") + self.assertEqual( + query.params["temporal"][3], "2016-10-12T10:55:07Z,2016-10-12T11:00:00Z" + ) def test_temporal_option_set(self): query = GranuleQuery() - query.temporal("2016-10-10T01:02:03Z", "2016-10-12T09:08:07Z", exclude_boundary=True) + query.temporal( + "2016-10-10T01:02:03Z", "2016-10-12T09:08:07Z", exclude_boundary=True + ) self.assertIn("exclude_boundary", query.options["temporal"]) self.assertEqual(query.options["temporal"]["exclude_boundary"], True) @@ -261,24 +292,24 @@ def test_orbit_number_encode(self): def test_day_night_flag_day_set(self): query = GranuleQuery() - query.day_night_flag('day') + query.day_night_flag("day") self.assertIn(self.day_night_flag, query.params) - self.assertEqual(query.params[self.day_night_flag], 'day') + self.assertEqual(query.params[self.day_night_flag], "day") def test_day_night_flag_night_set(self): query = GranuleQuery() - query.day_night_flag('night') + query.day_night_flag("night") self.assertIn(self.day_night_flag, query.params) - self.assertEqual(query.params[self.day_night_flag], 'night') + self.assertEqual(query.params[self.day_night_flag], "night") def test_day_night_flag_unspecified_set(self): query = GranuleQuery() - query.day_night_flag('unspecified') + query.day_night_flag("unspecified") self.assertIn(self.day_night_flag, query.params) - self.assertEqual(query.params[self.day_night_flag], 'unspecified') + self.assertEqual(query.params[self.day_night_flag], "unspecified") def test_day_night_flag_invalid_set(self): query = GranuleQuery() @@ -452,36 +483,39 @@ def test_valid_spatial_state(self): self.assertTrue(query._valid_state()) def _test_get(self): - """ Test real query """ + """Test real query""" query = GranuleQuery() - query.short_name('MCD43A4').version('005') + query.short_name("MCD43A4").version("005") query.temporal(datetime(2016, 1, 1), datetime(2016, 1, 1)) results = query.get(limit=10) self.assertEqual(len(results), 10) def test_stac_output(self): - """ Test real query with STAC output type """ + """Test real query with STAC output type""" # HLSL30: https://cmr.earthdata.nasa.gov/search/concepts/C2021957657-LPCLOUD query = GranuleQuery() - search = query.parameters(point=(-105.78, 35.79), - temporal=('2021-02-01', '2021-03-01'), - collection_concept_id='C2021957657-LPCLOUD' - ) + search = query.parameters( + point=(-105.78, 35.79), + temporal=("2021-02-01", "2021-03-01"), + collection_concept_id="C2021957657-LPCLOUD", + ) results = search.format("stac").get() feature_collection = json.loads(results[0]) self.assertEqual(len(results), 1) - self.assertEqual(feature_collection['type'], 'FeatureCollection') - self.assertEqual(feature_collection['numberMatched'], 2) - self.assertEqual(len(feature_collection['features']), 2) + self.assertEqual(feature_collection["type"], "FeatureCollection") + self.assertEqual(feature_collection["numberMatched"], 2) + self.assertEqual(len(feature_collection["features"]), 2) def _test_hits(self): - """ integration test for hits() """ + """integration test for hits()""" query = GranuleQuery() - query.short_name("AST_L1T").version("003").temporal("2016-10-26T01:30:00Z", "2016-10-26T01:40:00Z") + query.short_name("AST_L1T").version("003").temporal( + "2016-10-26T01:30:00Z", "2016-10-26T01:40:00Z" + ) hits = query.hits() self.assertEqual(hits, 3) @@ -514,7 +548,17 @@ def test_invalid_parameters(self): def test_valid_formats(self): query = GranuleQuery() - formats = ["json", "xml", "echo10", "iso", "iso19115", "csv", "atom", "kml", "native"] + formats = [ + "json", + "xml", + "echo10", + "iso", + "iso19115", + "csv", + "atom", + "kml", + "native", + ] for _format in formats: query.format(_format) @@ -548,7 +592,9 @@ def test_valid_concept_id(self): self.assertEqual(query.params["concept_id"], ["C1299783579-LPDAAC_ECS"]) query.concept_id(["C1299783579-LPDAAC_ECS", "G1441380236-PODAAC"]) - self.assertEqual(query.params["concept_id"], ["C1299783579-LPDAAC_ECS", "G1441380236-PODAAC"]) + self.assertEqual( + query.params["concept_id"], ["C1299783579-LPDAAC_ECS", "G1441380236-PODAAC"] + ) def test_token(self): query = GranuleQuery() @@ -575,3 +621,25 @@ def test_readable_granule_name(self): query.readable_granule_name(["*a*", "*b*"]) self.assertEqual(query.params[self.readable_granule_name], ["*a*", "*b*"]) +# Asegúrate de que no haya espacios antes de 'def' +def test_wkt_coordinate_order_warning(): + """ + Ensure a warning is raised when coordinates appear to be in the wrong order (span > 180). + """ + # Usamos coordenadas que cruzan el antimeridiano (span > 180) + flipped_coords = [(170, 10), (-170, 10), (-170, -10), (170, -10), (170, 10)] + query = GranuleQuery() + + # Verificamos que se dispare el UserWarning configurado en queries.py + with pytest.warns(UserWarning, match="longitude span is greater than 180 degrees"): + query.polygon(flipped_coords) + +def test_bounding_box_order_warning(): + """ + Verify warning for incorrect bounding box coordinate order. + """ + query = GranuleQuery() + + # Verificamos la alerta de cruce de antimeridiano definida en queries.py + with pytest.warns(UserWarning, match="crosses the antimeridian"): + query.bounding_box(10, 0, -10, 5) diff --git a/tests/test_issue_66.py b/tests/test_issue_66.py deleted file mode 100644 index 0ef5130..0000000 --- a/tests/test_issue_66.py +++ /dev/null @@ -1,31 +0,0 @@ -import unittest -import warnings -from cmr import queries - - -class TestIssue66(unittest.TestCase): - def test_wkt_coordinate_order_warning(self): - # 1. Usamos coordenadas que cruzan casi todo el mapa (Span > 180) - # De 170 a -170 hay una diferencia de 340 grados. - flipped_coords = [(170, 10), (-170, 10), (-170, -10), (170, -10), (170, 10)] - - query = queries.GranuleQuery() - - # 2. Ahora sí debería dispararse el UserWarning - with self.assertWarns(UserWarning) as cm: - query.polygon(flipped_coords) - - self.assertIn("longitude span is greater than 180 degrees", str(cm.warning)) - - def test_bounding_box_order_warning(self): - # Este ya pasaba, lo mantenemos igual - query = queries.GranuleQuery() - - with self.assertWarns(UserWarning) as cm: - query.bounding_box(10, 0, -10, 5) - - self.assertIn("crosses the antimeridian", str(cm.warning)) - - -if __name__ == "__main__": - unittest.main()