Lazy import of sklearn and xgboost

[yoavram]

Importing xgboost just before using it is needed is useful on systems where it is not installed.
Since installation of XGBoost can be tricky, I think this is a good idea.

[tlapusan]

Hi @yoavram, let me see if I've understood your issue.
You want to say that in current release version you need to have installed both scikit-learn and xgboost, even if you want to use only one of them?

[yoavram]

Yes it seems that when I import dtreeviz I get an error about bad xgboost install; when I moved the import (since I work with scikit learn) it works fine.
If you’d want to have it work without installing scikit learn, you’d probably need to try-except the import statement, but I haven’t done that.

[tlapusan]

I've checkout your PR and uninstalled xgboost.

If we cannot install xgboost or have problems importing it, we still will have issues.
Even if we have the error in pycharm (xgboost module cannot be found), we can still render scikit-learn visualizations... (because the way python interpreter works)

If we switch the ShadowXGBDTree initialization block and we want to render scikit-learn visualizations, we will receive the "ModuleNotFoundError: No module named 'xgboost'" exception.

Since get_shadow_tree() is called from any visualization methods.... I don't have an idea right now how to implement the "lazy" import. Any suggestions ?

[parrt]

I believe I did lazy import somewhere. hang on. Yep, i've done this kinda stuff. would this be helpful?

    if 'numpy' not in sys.modules:
        import numpy
    np = sys.modules['numpy']
    if not isinstance(data,np.ndarray):
        return " "

[yoavram]

No I think that’s an overkill. What I mean by “Lazy import” Is that you only import when you need it, not that you import if it hasn’t been imported. The latter is taken care of by Python automatically.

[yoavram]

@tlapusen I’ll take a closer look at what you wrote later.

[parrt]

Meaning that the user has to do the import? isn't that the way it is now?

[tlapusan]

hi @yoavram

do you continue with this or should we close this PR ?

[parrt]

@tlapusan I was thinking that we should do this conditional install that somebody fixed for me in https://github.com/parrt/tensor-sensor

That fixes what is required to be installed. Still, inside the code we have to avoid using types that don't exist. We tweaked the code also. See https://github.com/parrt/tensor-sensor/blob/master/tsensor/analysis.py#L459

[parrt]

@yoavram my understanding is that those imports would slow things down but I'm not sure it matters, depending on how many times you call the function. I think it's easiest if we asked for the class names and module names to check for types.

[tlapusan]

@parrt I'm gonna look on those conditional imports in the next days, thanks.

[parrt]

I'm laughing because I had no idea you could do conditional pip installs...

[tlapusan]

@parrt I've looked over conditional imports and found also pip docs about it. Like you said, we still need to check into code if the libraries are installed or not.
I suppose that the default installation (pip install dtreeviz) should install all requirements and the custom pip installations should be very well documented in readme.md page, else the people will be confused.

I started to work on lightgbm implementation and I would like to work on this fix (#99) later. (happy if someone else could do this meanwhile)

[parrt]

@tlapusan lightgbm is definitely more important :)

[parrt]

I think all we need is something like the following in the set up (from tensor-sensor):

    install_requires=['graphviz>=0.14.1','numpy','IPython', 'matplotlib'],
    extras_require = {'all': all_requires,
                      'torch': torch_requires,
                      'tensorflow': tensorflow_requires
                     },

then check for the modules:

if v.shape.__class__.__module__ == "torch": ...

poking around the web, it looks like people do this for conditional imports:

# pylint: disable=import-not-at-top
try:
  from monty.python.au import bruce
else:
  bruce = None

But, we still need to use the class/module name instead of checking the type in the code.

[tlapusan]

hi @yoavram, please follow this PR #115 if you are still interested in making xgboost or pyspark lazy imported.

[yoavram]

Sorry I fell off this issue. Hopefully someone can pick it up.

[parrt]

@tlapusan should we close this one in favor of #115? I'm trying to figure out which is the PR to focus on.

[parrt]

Closing since I merged #115.