feat(queries): implement coordinate order validation and UserWarnings for spatial queries (#66) by angel-cm-dev · Pull Request #105 · nasa/python_cmr

angel-cm-dev · 2026-04-04T04:16:16Z

🚀 Overview

This Pull Request addresses Issue #66 by implementing proactive validation for coordinate ordering in bounding_box and polygon methods within the GranuleQuery class.

The primary goal is to prevent common user errors—such as flipped latitude/longitude pairs—that lead to unintentional antimeridian crossings or empty API results. By providing immediate feedback via UserWarning, we improve the developer experience and reduce unnecessary load on the CMR API.

✅ Key Changes

Data Sanitization: Implemented explicit float() casting for all spatial parameters. This ensures type safety and prevents potential string-injection issues in the generated query parameters.
Bounding Box Intelligence: Added logic to detect if lower_left_lon > upper_right_lon or if latitudes are flipped, triggering a UserWarning with clear descriptive feedback.
Polygon Analysis: Added a check for the longitude span of polygons. If the span exceeds 180 degrees, a warning is issued to alert the user about possible coordinate flipping or intended antimeridian crossings.
Code Standards: The code has been refactored to comply with PEP 8 standards using Ruff. Clean code principles (SOLID) were applied to keep the methods readable and maintainable.

🧪 Testing & Verification

I have performed rigorous testing to ensure the robustness of these changes:

Unit Testing: Created tests/test_issue_66.py utilizing unittest.TestCase.assertWarns. This verifies that warnings are triggered exactly when expected and silenced during valid queries.
Integration (NASA UAT): Manually verified the fix against the NASA CMR UAT environment (cmr.uat.earthdata.nasa.gov).
- Verified that the generated URLs are correctly formatted (e.g., bounding_box=10.0,0.0,-10.0,5.0).
- Confirmed the library gracefully handles empty results from the API after the warning is issued.
Environment: Tests were executed on Python 3.14.3 on Windows 11, ensuring compatibility with the latest Python releases.

🔗 Related Issues

Closes #66

…box and polygon

frankinspace

Thank you for your contribution. Looks reasonable to me, just one comment to move the tests into the existing test case.

frankinspace · 2026-04-06T16:23:30Z

tests/test_issue_66.py

These can move into the test_granule.py test case.

chuckwondo

Thanks for the contribution. In the future, please avoid including various code formatting changes, as this PR is rather noisy with such changes, making it hard to easily see the meaningful changes you contributed.

chuckwondo · 2026-04-06T17:01:07Z

tests/test_issue_66.py

+from cmr import queries
+
+
+class TestIssue66(unittest.TestCase):


Should be no need to use unittest.TestCase. The methods can simply be top-level test functions. Then change the self.assertIn calls to standard assert statements.

chuckwondo · 2026-04-06T17:03:53Z

tests/test_issue_66.py

+
+class TestIssue66(unittest.TestCase):
+    def test_wkt_coordinate_order_warning(self):
+        # 1. Usamos coordenadas que cruzan casi todo el mapa (Span > 180)


English comments, please.

chuckwondo · 2026-04-06T17:04:00Z

tests/test_issue_66.py

+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.


chuckwondo · 2026-04-06T17:04:07Z

tests/test_issue_66.py

+
+        query = queries.GranuleQuery()
+
+        # 2. Ahora sí debería dispararse el UserWarning


chuckwondo · 2026-04-06T17:04:14Z

tests/test_issue_66.py

+        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


…tests to test_granule.py

angel-cm-dev · 2026-04-07T01:25:33Z

I've addressed all the feedback in the latest commit (78ff562):

All warning messages and internal comments have been translated to English.
Tests from test_issue_66.py were refactored into top-level functions and migrated to test_granule.py.
The redundant test_issue_66.py file has been deleted.
Verified that all 60 tests pass locally.

Please let me know if any further adjustments are needed. Ready for a new review!

frankinspace · 2026-04-08T15:17:31Z

tests/test_granule.py

+    """
+    Ensure a warning is raised when coordinates appear to be in the wrong order (span > 180).
+    """
+    # Usamos coordenadas que cruzan el antimeridiano (span > 180)


English comments please

frankinspace · 2026-04-08T15:17:56Z

tests/test_granule.py

+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)


Indentation appears to be incorrect

That's based on an earlier comment I made, indicating that these test functions can be top level test functions rather than within the test case class, but that was before I saw that all of the tests in this file are within the class.

I'm fine with these new functions also being methods within the class, but we should probably separate such methods into top level functions (in a separate PR) because most of the test methods in this class do not require vcrpy, so placing them within the class doesn't make sense (although does no harm either).

Ah I see your point. Agreed it can be refactored

chuckwondo · 2026-04-08T19:27:47Z

tests/test_granule.py

+    """
+    query = GranuleQuery()
+
+    # Verificamos la alerta de cruce de antimeridiano definida en queries.py


English comments, please.

chuckwondo · 2026-04-08T19:28:00Z

tests/test_granule.py

+    flipped_coords = [(170, 10), (-170, 10), (-170, -10), (170, -10), (170, 10)]
+    query = GranuleQuery()
+
+    # Verificamos que se dispare el UserWarning configurado en queries.py


English, please.

chuckwondo · 2026-04-08T19:28:47Z

tests/test_granule.py


        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'


English, please. Also, please add a blank line above to separate it from the preceding method.

chuckwondo · 2026-04-08T19:31:35Z

tests/test_granule.py

+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)


That's based on an earlier comment I made, indicating that these test functions can be top level test functions rather than within the test case class, but that was before I saw that all of the tests in this file are within the class.

I'm fine with these new functions also being methods within the class, but we should probably separate such methods into top level functions (in a separate PR) because most of the test methods in this class do not require vcrpy, so placing them within the class doesn't make sense (although does no harm either).

chuckwondo · 2026-04-08T19:38:37Z

cmr/queries.py

+        self.params["bounding_box"] = f"{ll_lon},{ll_lat},{ur_lon},{ur_lat}"
        return self
-
+        


Suggested change

feat(queries): add user warnings for flipped coordinates in bounding_…

f83ad95

…box and polygon

frankinspace suggested changes Apr 6, 2026

View reviewed changes

tests/test_issue_66.py Outdated

Copy link
Copy Markdown

Contributor

frankinspace Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These can move into the test_granule.py test case.

chuckwondo requested changes Apr 6, 2026

View reviewed changes

refactor: address reviewer feedback - translate messages and migrate …

78ff562

…tests to test_granule.py

frankinspace suggested changes Apr 8, 2026

View reviewed changes

chuckwondo requested changes Apr 8, 2026

View reviewed changes

		from cmr import queries


		class TestIssue66(unittest.TestCase):


		query = queries.GranuleQuery()

		# 2. Ahora sí debería dispararse el UserWarning

		self.params["bounding_box"] = f"{ll_lon},{ll_lat},{ur_lon},{ur_lat}"
		return self

Conversation

angel-cm-dev commented Apr 4, 2026

🚀 Overview

✅ Key Changes

🧪 Testing & Verification

🔗 Related Issues

Uh oh!

frankinspace left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chuckwondo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

angel-cm-dev commented Apr 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

frankinspace Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

frankinspace Apr 8, 2026 •

edited

Loading