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

Dependency considerations #64

Open
dshean opened this issue Nov 19, 2024 · 1 comment
Open

Dependency considerations #64

dshean opened this issue Nov 19, 2024 · 1 comment

Comments

@dshean
Copy link
Member

dshean commented Nov 19, 2024

@rhugonnet
Copy link
Member

Looking good! Most dependencies seem needed and not build up the graph too much (apart from maybe scikit-gstat in xdem, that we might make optional soon).

To summarize what I was saying: The main idea behind making some dependencies optional is to not impose them on your user (like for scikit-gstat imposed by xdem above), and how the dependencies of that other package also create more dependencies (in this case scikit-gstat just requires scipy that's already used, but could be different for other packages), and avoid blowing user's environments bit by bit.

So the choice depends on both on the extra dependencies added by the other package, and on how much that other package is used in your own (i.e. only specific to a single module/functionality, or used across all modules?) and can potentially be left out without affecting the general behaviour of your package.

If the usage of a package is specific to a single functionality (like sliderule is to your asp_plot.altimetry module right now), then it might make sense to make it optional simply by adding something like this at the top of your script:

try:
    import sliderule

    _has_sliderule = True
except ImportError:
    _has_sliderule = False

Then when calling the required functionality:

  if not _has_sliderule:
            raise ValueError("Optional dependency needed. Install 'sliderule'")

(ideally not a ValueError but a custom subclass with name like OptionalDependencyError or something, but ValueError does the job)

Definitely not a priority for you right now (like you labeled), but something to keep in mind! 😉

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

No branches or pull requests

3 participants