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

Decide, document (and rethink?) optional dependency handling #655

Open
dstansby opened this issue Nov 21, 2024 · 3 comments
Open

Decide, document (and rethink?) optional dependency handling #655

dstansby opened this issue Nov 21, 2024 · 3 comments

Comments

@dstansby
Copy link
Contributor

Currently the approach to handling optional dependencies is to not define codec classes if the dependency is not present (e.g., see #637). This leads to a poor user experience when you want to use an optional codec, but haven't installed the optional dependency: if you try and import the codec, you get an import error without any information that to fix the error you have to install a specific package: #526

I propose that we switch to a model where instead instantiating a codec class where an optional dependency is missing raises a helpful error message.

At some point I will create a PR with a concrete implementation of this change to help see what it would mean, but please share opinions on if there are other/better ways to improve user experience around optional dependency handling.

@dstansby
Copy link
Contributor Author

Pinging @jakirkham to see if you have any thoughts, because I know you've been tidying this up recently.

@jakirkham
Copy link
Member

We have allowed a proliferation of increasingly experimental or custom codecs in Numcodecs. However this has come at substantial maintenance cost as they don't always get updated in a timely fashion or incompatible with Python or NumPy releases. So requires lots of finessing in requirements, warnings/errors, and CI.

We have tried to manage this by making them optional and guarding them in various ways. Though doing this correctly is not always straightforward for other contributors.

Think the answer has to be moving these out into separate repos/libraries. IOW Numcodecs extensions that users can install or not.

@dstansby
Copy link
Contributor Author

dstansby commented Dec 3, 2024

I propose that we switch to a model where instead instantiating a codec class where an optional dependency is missing raises a helpful error message.

So I tried this out over at #666, and it didn't work because (at least) ZFPY has default arguments in the class method signatures that require values from the zfpy package.

With the current way codecs are namespaced, ie numcodecs.{CODEC} I don't think there's a nicer way to warn or error to users if a dependency is missing. So I think the choice is:

  1. Change namespacing so codecs are in numcodecs.{codec-sub-module}.{CODEC}, forcing users to import the submodule to use it (and removing the imports from numcodecs/__init__.py
  2. Keep the status quo where codecs just silently don't exist if a dependency isn't installed

I don't think I have particular strong opinions either way - if I was writing from scratch I'd definitely go for 1), but given the pain of changing namespacing perhaps we should just stick with 2)?

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