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

optional dependencies and third-party index implementations #43

Open
keewis opened this issue Jan 29, 2024 · 4 comments
Open

optional dependencies and third-party index implementations #43

keewis opened this issue Jan 29, 2024 · 4 comments

Comments

@keewis
Copy link
Collaborator

keewis commented Jan 29, 2024

We currently import all index implementations to create a mapping of registered grid types. However, this currently forces us to require all DGGS support libraries to be installed (healpy and h3ronpy, currently). This is problematic because it means we pull in potentially unused libraries, and in the case of healpy this also means that we cannot test and support windows (since healpy doesn't).

In order to avoid the above and to allow third-party index implementations we should think about creating a plugin system.

@benbovy
Copy link
Member

benbovy commented Feb 6, 2024

Agreed for making dependencies optional.

Regarding third-party index implementations this might be a bit premature a this stage, though. There aren't tons of DGGS (systems, implementations and/or backend libraries) in the wild and it would add some friction if we cannot easily move forward and break the API of the base DGGS index.

@keewis
Copy link
Collaborator Author

keewis commented Feb 6, 2024

Very true, my idea was to redesign with third-party implementations in mind, even if we don't publicly expose the interface (I wouldn't put too much energy into getting that to work, though, just make sure we don't need another refactor). That would make exposing it very easy, but without freezing the API at first.

@keewis
Copy link
Collaborator Author

keewis commented Feb 6, 2024

regarding the DGGS implementations: I believe you're right, though that leaves us with the question of where these should live. @allixender has voiced interest in index implementations backed by dggrid, would you put these directly in xdggs or in a separate package?

@keewis
Copy link
Collaborator Author

keewis commented Nov 5, 2024

since there has been an increased interest in third-party index implementations, here's how you'd do it:

  1. create one xdggs.DGGSInfo subclass per grid system
  2. create one xdggs.DGGSIndex subclass per grid system
  3. decorate it with register_dggs("<grid-name>")

For example:

from dataclasses import dataclass
from xdggs import DGGSInfo, DGGSIndex
from xdggs.utils import register_dggs

@dataclass(frozen=True)
class S2Info(DGGSInfo):
    level: int
    # any additional parameters here

    # need to implement `from_dict`, `to_dict`, `cell_ids2geographic`, `geographic2cell_ids`, and `cell_boundaries`
    # can make use of `translate_parameters` if multiple conventions exist
    ...

@register_dggs("s2")
class S2Index(DGGSIndex):
    # implement at least `from_variables`, `_replace` and `_repr_inline_`
    ...

I don't think we need to include this in #75, but at some point we need to add a guide on this to the documentation.

cc @acocac

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