-
Notifications
You must be signed in to change notification settings - Fork 229
Add optional dependency management #537
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
Changes from 21 commits
d8d6c6b
2915ccc
702c11e
317806a
0b8f869
a56e6ad
fc7618b
1196c3b
642de25
a7c883d
99d697a
94aaf32
0df1ba6
3d7a122
8410242
026133f
8407df4
2ff55ae
b9385d7
5bc3119
60b97c6
ece444f
1e211c4
9c31d66
5e8ed81
67c81b4
b9458eb
3375873
6426b61
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,12 @@ | ||
import numpy as np | ||
from typing import Callable, Dict, Optional, Union | ||
from alibi_detect.utils.frameworks import has_pytorch, has_tensorflow, has_sklearn | ||
from alibi_detect.utils.frameworks import has_pytorch, has_tensorflow, \ | ||
BackendValidator | ||
from alibi_detect.base import DriftConfigMixin | ||
|
||
if has_sklearn: | ||
from sklearn.base import ClassifierMixin | ||
from alibi_detect.cd.sklearn.classifier import ClassifierDriftSklearn | ||
|
||
from sklearn.base import ClassifierMixin | ||
from alibi_detect.cd.sklearn.classifier import ClassifierDriftSklearn | ||
|
||
if has_pytorch: | ||
from torch.utils.data import DataLoader | ||
|
@@ -147,12 +148,12 @@ def __init__( | |
self._set_config(locals()) | ||
|
||
backend = backend.lower() | ||
if (backend == 'tensorflow' and not has_tensorflow) or (backend == 'pytorch' and not has_pytorch) or \ | ||
(backend == 'sklearn' and not has_sklearn): | ||
raise ImportError(f'{backend} not installed. Cannot initialize and run the ' | ||
f'ClassifierDrift detector with {backend} backend.') | ||
elif backend not in ['tensorflow', 'pytorch', 'sklearn']: | ||
raise NotImplementedError(f"{backend} not implemented. Use 'tensorflow', 'pytorch' or 'sklearn' instead.") | ||
BackendValidator( | ||
backend_options={'tensorflow': ['tensorflow'], | ||
'pytorch': ['pytorch'], | ||
'sklearn': ['sklearn']}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In fact, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure about your comment about the full import check vs setting the boolean flags, probably would need to see the code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just that here we try and import |
||
construct_name=self.__class__.__name__ | ||
).verify_backend(backend) | ||
|
||
kwargs = locals() | ||
args = [kwargs['x_ref'], kwargs['model']] | ||
|
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.
should this be
'pytorch': ['torch']
? check elsewhere whereBackendValidator
is invoked too and the docstring forBackendValidator
also.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 dependencies' presence is checked by the
HAS_BACKEND
dict
which has keypytorch
. Thepip install
message on the other hand is generated from theERROR_TYPES
which maps totorch
which is also the name of our dependency bucket. So the error message is:I kept this in line with the error messages that were being generated prior to the addition of the BackendValidator.
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.
ok makes sense, but probably would be great to add some comments in the code around this
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.
@mauicv did you add the comments @jklaise is referring to here?
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.
I've updated the comment here to be clearer about the behaviour!