Skip to content
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

creating templates #35

Open
keewis opened this issue Dec 4, 2023 · 5 comments
Open

creating templates #35

keewis opened this issue Dec 4, 2023 · 5 comments

Comments

@keewis
Copy link
Collaborator

keewis commented Dec 4, 2023

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
Copy link
Member

benbovy commented Dec 12, 2023

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
Copy link
Collaborator Author

keewis commented Dec 12, 2023

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

@keewis keewis mentioned this issue Dec 21, 2023
1 task
@keewis
Copy link
Collaborator Author

keewis commented May 14, 2024

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
Copy link
Member

benbovy commented May 22, 2024

for another idea, how about we move all the conversion methods to the grid objects from #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
Copy link
Collaborator Author

keewis commented May 29, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants