xarray-contrib / xdggs

Xarray extension for DGGS
Apache License 2.0
54 stars 9 forks source link

creating templates #35

Open keewis opened 9 months ago

keewis commented 9 months ago

For regridding (but not just there), it would be useful to have a way to "rasterize" a given shape, typically a bounding box (segmentized if we truly want to follow the small circles), and get a Dataset with the resulting cell ids back. I believe that this could live in xdggs, but I'm not sure how to design the API.

However, if I were able to redesign the current API, I'd create "grid" objects (basically dataclasses) that contain the dggs parameters and have both the indexes and the new functions consume those objects. In a way, we'd have specialized CRS objects for DGGS (I'm not proposing to actually make them CRS objects, though). For example, the index __init__ would become:

class DGGSIndex:
    def __init__(self, cell_ids: Any | PandasIndex, dim: str, grid: Grid):
        ...

(or reorder and expect the grid parameter first)

This would make changes to the parameter names and different competing parameter definitions (see #34) much easier to handle, and in general testing might also become easier.

benbovy commented 8 months ago

Agreed this would be useful! Also cleaner for validating DGGS parameter values and generally for handling DGGS specification (with respect to DGGS standards or conventions) and refactor it more easily later if needed.

Grid sounds a bit too generic to me. What about DGGSParams? DGGSSpecs?

keewis commented 8 months ago

Indeed, I'm not particularly attached to name. Maybe just DGGS, which would be pretty close in meaning to CRS? Or DGGSInfo?

keewis commented 3 months ago

for another idea, how about we move all the conversion methods to the grid objects from #39? All we really need are the DGGS parameters and the cell ids, which don't have to come from the index itself. Thus, not having these on the index would make for a cleaner API.

benbovy commented 3 months ago

for another idea, how about we move all the conversion methods to the grid objects from https://github.com/xarray-contrib/xdggs/pull/39?

Yes this makes sense.

Alternatively, we could have a unique class similar to pyproj.Proj for handling the conversions. This would make the API cleaner while keeping simple the DGGSInfo classes (i.e., basic dataclasses holding metadata, similar to pyproj.crs.CRS). From an implementation point of view this may make things more complicated, though. I guess that unlike converting between different CRSes in pyproj, the DGGS conversion internal code may greatly differ from one backend to another.

Either way, a unified API for DGGS conversion might be reused in other contexts than Xarray and may thus eventually be factored out of xdggs.

keewis commented 3 months ago

Looking at this again, I agree that the conversions are too different from one another to be consolidated in a single class. Furthermore, dependency management would become really tricky, since every implementation would need different dependencies.

However, if we move the conversion methods to the grid object, that means that most index implementations are identical (as long as we don't need a different index type, e.g. if we have more than a single horizontal dimension). So ideally we could just define a standard implementation in the base class, and the individual index implementations would only differ in their reprs and type hint overrides.