Add a new method that filters by geometry

[framunoz]

I would like to include a new method for the ImageCollectionAccessor class that filters by geometry.

Unlike the method filterBounds(geometry, errorMargin), this one filters the collection if the geometry parameter intersects with an image. My proposal is to include the filterByGeometry(geometry) method, which will include the image to the image collection if geometry is contained in the image. To summarize, the difference between the two filters is that the first one is an intersection, while the second one is an inclusion.

[fitoprincipe]

Hi @framunoz! Thanks for the proposal 😃 We should discuss if:

1. we should create a function to replace a "one line code": filtered = ee.Filter.isContained('.geo', geometry)
   I agree that many users don't know that they can use the .geo property to refer to the geometry. If we decide to code it:
2. Is the name meaningful? filterByGeometry? filterContained? filterContainedGeometries?

What do you think @12rambau ?

[12rambau]

I think filterContained is clearly the best option, now the question is should it be a ee.Filter extension or a ImageCollection one ?
I would vote for ee.Filter to make it available to all types of collections.

[fitoprincipe]

I would also vote for ee.Filter but my question is, can we extend a filter?? it will not operate over itself but over the collection, so self._obj would not be useful 🤔 right? I need to think deeper on how to implement this... (thinking aloud) my quick first solution would be to extend ee.Collection.filter (both Image and Feature) to be able to pass custom filters.. actually I have though on this long ago, not only for filters but also for reducers... any function that takes a collection and returns a new collection could be a filter, and any function that takes a bunch of data and returns a value (or many) could be a reducer.... (though on this when coding the medoid reduction).

Coming back to filtering, the first thing to do would be to identify whether it's a native filter (ee.Filter) or a custom one. Since filtering is a common operation done inside mapping functions, we should not rely on client-side methods...

[framunoz]

Hi! Thank you very much for discussing my proposal.

With respect to the discussion, it also makes a lot of sense to me that the new proposal is part of a filter rather than an extension of a collection of images.

Regarding for the question:

but my question is, can we extend a filter?? it will not operate over itself but over the collection, so self._obj would not be useful 🤔 right?

I don't think it's that uncommon to have a method that doesn't modify the object. Most of the methods in ee.filter are static methods, and are intended more to be alternative constructors of an ee.Filter, and never use self. In our case, I think we should also follow the same convention, even though we don't use that information.

In that case, a tentative implementation (inspired in the original implementation) would be:

from abc import staticmethod

ErrorMargin = ...  # Typing for error mrgin, maybe inspired in ee._arg_types

class FilterAccessor:
  ...

  @staticmethod
  def filterContained(geometry: ee.Geometry, maxMargin: ErrorMargin | None = None) -> ee.Filter:
    return ee.Filter.isContained('.geo', geometry, maxMargin=maxMargin)

I include the maxMargin argument in this case. Therefore, a use case would be:

ic = (
  ee.ImageCollection(...)
  # Concatenate any filter or pre-process
  .filter(ee.Filter.geetools.filterContained(geometry))
)

In this case, would it still be convenient to call it filterContained?, since that name would be more convenient if it were part of ImageCollection.

[fitoprincipe]

In this case, your solution could (and should) work because the custom filter returns an ee.Filter. As long as we keep this pattern it will work, I guess. My question was more related to a deeper level of customization, where you take a "list" of data and return a subset of it. Same happens with reducers; it'd be easier to implement a custom reducer that returns an ee.Reducer than a custom "function" that returns the processed data.

Regarding the question on the name, I see your point. I think we should specify that it's working on geometries, so, IMO filterContainedGeometries would fit better.

[12rambau]

On my end I think you can drop the filter as it's already specified by the class and make it shorter. contained is sufficient to me but if you really need to convey the spatial (I guess opposed to time) container then what about geocontained ?

[fitoprincipe]

You're right @12rambau, since it'll be in the ee.Filter namespace the filter part is redundant. However, geocontained doesn't convince me 🤔. ee.Filter.geetools.geometryContained ??

[framunoz]

makes sense to me, I like geoContained because it is more similar to isContained. It could be geometryContained to avoid abbreviations.