-
Notifications
You must be signed in to change notification settings - Fork 21
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
CMR-10146: Making sure validBbox compares numbers and not strings #378
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #378 +/- ##
=======================================
Coverage 88.22% 88.22%
=======================================
Files 24 24
Lines 1180 1180
Branches 261 261
=======================================
Hits 1041 1041
Misses 139 139 ☔ View full report in Codecov by Sentry. |
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.
These lines are covered in CodeCov, so I'd expect a test update to go along with this code change. Because they all still pass maybe the input value to a test somewhere is wrong and it is passing numbers where this was a bug because real values are strings?
If it isn't a bad existing test we should be sure to add a new test that covers when these values are strings. That new test should be pretty easy to add as a unit test on this file, which would be better than trying to track down where in the existing tests this code is called.
Adjusted the input values so that unit tests can be written. |
* CMR-10146: Making sure validBbox compares numbers and not strings * CMR-10146: Adding comments * CMR-10146: Adjusting the bbox types and adding tests * CMR-10146: Making test prettier
Overview
What is the feature?
CMR-STAC's validBbox function may be passed an array of strings. In this case, the comparison between points does a comparison with strings. When negative numbers are used, comparing negative numbers as strings doesnt work properly.
What is the Solution?
Need to cast the comparison as numbers so numerical comparison is used instead of string comparison.(for negative numbers)
What areas of the application does this impact?
When a bbox array of string is passed in.
Testing
Reproduction steps
Can use the following url to test:
https://cmr.earthdata.nasa.gov/stac/LPCLOUD/search?bbox[0]=-122.09&bbox[1]=39.89&bbox[2]=-122.03&bbox[3]=39.92&datetime=2024-01-01T00:00:00Z/2024-09-24T00:00:00Z&limit=1&collections=HLSL30_2.0
Currently getting an error as follows:
{"errors":["BBOX must be in the form of 'bbox=swLon,swLat,neLon,neLat' with valid latitude and longitude."]}
Attachments
Showing R STAC script not working Showing R STAC script workingChecklist