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

[WIP] Check compatible optional dependency versions #714

Merged
merged 4 commits into from
Jul 12, 2022

Conversation

mauicv
Copy link
Collaborator

@mauicv mauicv commented Jul 7, 2022

What is this:

This draft PR addresses #709 and adds checks for compatibility of optional dependencies in import_optional. Note that at the moment this functionality just checks that the correct submodels are present in the imported dependency. This solves the issue described in #709 where Shap throws an error on account of the version installed in Kaggle having an incompatible module structure.

Todo:

  • Rewrite warning to reflect incompatible versions not just uninstalled dependency

Open Questions:

Should we be checking the version compatibility explicitly instead?

@codecov
Copy link

codecov bot commented Jul 7, 2022

Codecov Report

Merging #714 (a948732) into master (5b7931d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #714   +/-   ##
=======================================
  Coverage   81.06%   81.06%           
=======================================
  Files         105      105           
  Lines       11793    11794    +1     
=======================================
+ Hits         9560     9561    +1     
  Misses       2233     2233           
Impacted Files Coverage Δ
alibi/utils/missing_optional_dependency.py 94.28% <100.00%> (+0.16%) ⬆️

@jklaise
Copy link
Contributor

jklaise commented Jul 12, 2022

LGTM.

I don't think we should explicitly check versions as that would introduce a lot of extra complexity and upkeep, the version requirements are clearly defined in the relevant files and it is the job of the pip resolver to bring the right ones in (I'm surprised that pip install alibi on the Kaggle kernel didn't update the shap version to a compatible one - perhaps it's some peculiarity with the Kaggle environment?).

@mauicv
Copy link
Collaborator Author

mauicv commented Jul 12, 2022

LGTM.

I don't think we should explicitly check versions as that would introduce a lot of extra complexity and upkeep, the version requirements are clearly defined in the relevant files and it is the job of the pip resolver to bring the right ones in (I'm surprised that pip install alibi on the Kaggle kernel didn't update the shap version to a compatible one - perhaps it's some peculiarity with the Kaggle environment?).

Yeah agreed re maintenance. I assumed that running pip install alibi[shap] would suffice but for some reason, it didn't seem to fix the issue. Although, just took a look and since the original error it seems like shap==0.41.00 is now the default in Kaggle!

@mauicv mauicv merged commit e790f52 into SeldonIO:master Jul 12, 2022
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

Successfully merging this pull request may close these issues.

2 participants