-
Notifications
You must be signed in to change notification settings - Fork 224
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
Add optional dependency management #537
Conversation
Merging 0.10.0rc1 back to master so that we can include the optional dependency work (#537) in the final 0.10.0 release.
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
* Add BackendValidator class * Protect ad, od and cd and other API objects from tensorflow and torch optional dependency import errors * Update import statements in notebooks
5047c71
to
a7c883d
Compare
* Add pydantic validator to check model wrt to the selected backend * protect `saving.tensorflow` imports with import_optional * Make error message more descriptive for preprocessing function missing from registry * Update README.md * Make `Detector` and `ConfigurableDetector` protocols for typing config-driven save and load functionality Co-authored-by: Ashley Scillitoe <ashley.scillitoe@seldon.io>
* Make prophet an optional dependency * Add check for missing prophet in save_detector_legacy
* Add pip install instructions for optional dependencies to docs * Update README.md
check_correct_dependencies(tensorflow_utils, dependency_map, opt_dep) | ||
|
||
|
||
def test_pytorch_utils_dependencies(opt_dep): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other test functions are all named test_torch_...
rather than test_pytorch_...
. Change for consistency?
@@ -0,0 +1,114 @@ | |||
"""Functionality for optional importing | |||
This module provides a way to import optional dependencies. In the case that the user imports some functionality from | |||
alibi that is not usable due to missing optional dependencies this code is used to allow the import but replace it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change alibi
to alibi_detect
? Maybe worth a quick grep to check if any other places where comments have been copied from the alibi
implementation?
Commenting here as can't comment on empty file. For |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Add changelog for optional dependency work
Hi. Do you have any plan to add saving/loading functionality for PyTorch model/backend? |
Hi @bluepark-sk, we sure do. Time permitting, I will be beginning work on PyTorch save/load this week (for drift detectors only)! |
@ascillitoe Thank you! I'll be waiting~~ |
TODO:
feature branch for numba optional dependenciesImage branch (see notion)What is this PR?
This PR addresses optional dependency import functionality for Alibi Detect. The core behaviour this adds is to throw errors informing the user if they haven't installed the necessary optional dependency for the functionality they wish to use. These errors are thrown at use time rather than at import.
There are three behaviours in general:
backend
should be one of'tensorflow'
or'torch'
but not neither.This PR addresses behaviours 1 and 3 above.
Note:
In order to manually test alibi-detect in each of the different environments developers can use:
and this will give them a REPL with the
<env>
optional dependencies installed.For example:
make repl tox-env=torch
will give them the torch optional dependency environment REPL.The main changes are:
1. MissingDependency
The MissingDependency metaclass is used to allow imports that have missing dependencies. If these imports are used later they will throw an error.
Implementation
The basic pattern is to replace constructs that require uninstalled dependencies with something like:
We also use an import function which should be used throughout by developers when importing constructs dependent on optional dependencies in order to ensure consistent behaviour. This looks roughly like:
The above is called with:
The above raises an error if the user attempts to access an attribute or call the object.
UsefulClass
object is missing optional dependencies.pip install alibi-detect[optional-dependency]
.2. Refactoring:
The idea here is wherever the is an object that is dependent on an optional install the relevant functionality should be fenced off into a single file or collection of files. Access to that object should be via the
__init__.py
file. Within the__init__.py
we will implement theMissingDependency
pattern mentioned above.Note on Type checking issue:
import_optional
function returns an object instance rather than an object. This will cause typechecking to failif not all optional dependencies are installed. Because of this we also need to 1. Conditionally import the true object
dependent on
TYPE_CHECKING
and 2. Use forward referencing within typing constructs such asUnion
. We use forwardreferencing because in a user environment the optional dependency may not be installed in which case it'll be replaced
with an instance of the MissingDependency class. This will throw an error when passed to
Union
. SeeCONTRIBUTING.md
note for more details and example.3. Tests:
The tests import all the named objects from the public API of alibi and test that they throw the correct errors if the relevant optional dependencies are not installed. If these tests fail, it is likely that:
functionality should work for the default alibi-detect install. If this is not the case you should add the exported object name to the
dependency_map
in the relevant test.MissingDependency
class. In this case, see the docs string for theutils.missing_dependency
m1odule.Notes:
tox
from the terminal. The environments will take a long time to build the first time but afterwards, this should be a quick process.setup.cfg
file and add atestenv
environment as well as to theextra-requires.txt
file__init__.py
files.See also Further Notes