Support multiple-point searches

[mfisher87]

https://cmr.earthdata.nasa.gov/search/site/docs/search/api.html#c-point

#490

[jhkennedy]

Possibly related to #504

[mfisher87]

cc @Sherwin-14

[Sherwin-14]

@mfisher87 Hey, needed some clarity over this. I went through the search_data function and search.py file, the file has a point method implemented already so wouldn't that suffice? If no, what can be the exact steps to support multiple point searches and also the likes of polygon and line as referenced here.

[Sherwin-14]

Looking at #504 do we need to do a similar task ( improving polygon coordinate handling) as the OP suggested in the discussion?

[chuckwondo]

I suggest opening an enhancement request against python_cmr. That's where all such CMR search enhancement requests should be made first, because the ultimate goal is to remove all duplicate CMR search functionality from earthaccess.

The reason we have duplicate functionality in earthaccess is that we previously were unable to get enhancements made to python_cmr. Now that we have a responsive team on python_cmr, we should start by making CMR search enhancement requests there.

We might still end up duplicating code here until python_cmr reaches parity with the duplicated functionality here, but in order to better facilitate being able to remove the duplicated functionality here, we must be sure to start adding new functionality to python_cmr first, then duplicate it here, as necessary, until we get to the day when we can rip out the duplicate code here.

[mfisher87]

the file has a point method implemented already so wouldn't that suffice

earthaccess/earthaccess/search.py

Lines 860 to 873 in be1ec48

	def point(self, lon: FloatLike, lat: FloatLike) -> Self:
	"""Filter by granules that include a geographic point.
	Parameters:
	lon: longitude of geographic point
	lat: latitude of geographic point
	Returns:
	self
	Raises:
	ValueError: `lon` or `lat` cannot be converted to a float.
	"""
	return super().point(lon, lat)

This method only supports a single point currently, but we want to support any number of points to reach parity with the CMR API we're using under the hood. As chuck mentioned, the python_cmr project is now accepting PRs and feature requests 💯

[Sherwin-14]

That means I might need to open up a feature request in python_cmr repo? I thought I should try there so maybe we can discuss the solution here and then open up a PR there?

[chuckwondo]

the file has a point method implemented already so wouldn't that suffice

This method only supports a single point currently, but we want to support any number of points to reach parity with the CMR API we're using under the hood. As chuck mentioned, the python_cmr project is now accepting PRs and feature requests 💯

Technically, this method supports whatever python_cmr supports, since it simply invokes the superclass method (which is from python_cmr), so if python_cmr were to support multiple points, so would earthaccess.

It turns out that python_cmr currently supports only 1 point, so please open an enhancement request in python_cmr to support multiple points: https://github.com/nasa/python_cmr/blob/develop/cmr/queries.py#L491-L506

[mfisher87]

Technically, this method supports whatever python_cmr supports, since it simply invokes the superclass method (which is from python_cmr), so if python_cmr were to support multiple points, so would earthaccess.

Maybe I'm missing something, but it looks to me like there's more to this. When python_cmr begins to support two argument forms for backwards compatibility, e.g. lon: FloatLike, lat: FloatLike (current) and points: list[tuple[FloatLike, FloatLike]] (to enable this feature), earthaccess will continue to expect the former form, only capable of passing lon and lat to python_cmr's superclass method. There wouldn't be a way to pass in the new single-argument list-of-points form! We still need to overload the function signature on our end.

[chuckwondo]

Technically, this method supports whatever python_cmr supports, since it simply invokes the superclass method (which is from python_cmr), so if python_cmr were to support multiple points, so would earthaccess.

Maybe I'm missing something, but it looks to me like there's more to this. When python_cmr begins to support two argument forms for backwards compatibility, e.g. lon: FloatLike, lat: FloatLike (current) and points: list[tuple[FloatLike, FloatLike]] (to enable this feature), earthaccess will continue to expect the former form, only capable of passing lon and lat to python_cmr's superclass method. There wouldn't be a way to pass in the new single-argument list-of-points form! We still need to overload the function signature on our end.

No need to do that. You just need to be able to call point once per point and have the method add to a list of points on each call instead of having each call simply set the 1 and only point that it currently keeps track of.

[chuckwondo]

In case you're wondering, yes, there are existing functions that behave in this manner, where you can call them multiple times to tack on additional values instead of overwriting a sole value. For example: https://github.com/nasa/python_cmr/blob/develop/cmr/queries.py#L396

[Sherwin-14]

I opened up a issue on python_cmr nasa/python_cmr#72

[mfisher87]

Thanks all for the explanation and for the paperwork! :)

[mfisher87]

💯 Awesome work getting this feature merged into python-cmr @Sherwin-14 and @chuckwondo! nasa/python_cmr#73