Update Model_Understanding documentation for plotly use on Jupyter Lab #1108
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1108 +/- ##
==========================================
- Coverage 99.91% 99.91% -0.01%
==========================================
Files 195 195
Lines 11159 11257 +98
==========================================
+ Hits 11150 11247 +97
- Misses 9 10 +1
Continue to review full report at Codecov.
|
evalml/model_understanding/graphs.py
Outdated
| @@ -105,6 +105,7 @@ def graph_precision_recall_curve(y_true, y_pred_proba, title_addition=None): | |||
| plotly.Figure representing the precision-recall plot generated | |||
| """ | |||
| _go = import_or_raise("plotly.graph_objects", error_msg="Cannot find dependency plotly.graph_objects") | |||
| import_or_raise("ipywidgets", error_msg="Cannot find dependency ipywidgets") | |||
There was a problem hiding this comment.
This might limit the use of our graphs for only notebook environments - not too sure if this is an issue but I can see use cases where we need to return the ploty figures and display them in a web environment. I think maybe the doc edit is good enough for users that want to use our graphs in a notebook environment.
There was a problem hiding this comment.
Ah great point. Thanks @jeremyliweishih, I'll update this.
There was a problem hiding this comment.
Maybe we can try to use get_ipython like we do in AutoMLSearch to decide whether or not to surface this error? Maybe we also don't need to raise, we can just issue a warning about possible missing dependencies? @jeff-hernandez was not seeing any plots show up in Jupyter Lab and it's really confusing when it happens (See this PR).
There was a problem hiding this comment.
My current implementation uses a jupyter_check function that does a try-except with get_ipython, which would return True if the call succeeds, or if it throws an exception, False. Unfortunately, I'm not able to test whether this call works through unittests since I cannot patch get_ipython -- I get the following error: ModuleNotFoundError: No module named 'Ipython'. Because of this, I can't test whether or not ipywidgets is installed or not properly, since I only make this call if jupyter_check returns True.
@freddyaboulton suggested that we could get coverage for this if we add ipython to test-requirements.txt, but we weren't sure if this is the best course of action.
bchen1116
changed the title
There was a problem hiding this comment.
@bchen1116 I think this is almost ready for merge! I have a couple of suggestions for how to increase test coverage.
| """ | ||
| try: | ||
| get_ipython() | ||
| return True |
There was a problem hiding this comment.
I think we can get to a point where this is the only line that's not covered by unit tests.
There was a problem hiding this comment.
@bchen1116 This looks good to merge! The only thing I can suggest is to remove the category=ImportWarning so that the warning is displayed to the user.
For those following, I think it's ok to merge this PR without passing the codecov check because only one line is not covered and we can cover it if we add IPython as a test requirement (so that we can mock the get_ipython function) but I'm not sure if we need to add such a big dependency.
I verified this works as expected in Jupyter Lab (once we remove the warning category)!
Fix #1040
Plots doesn't show up on Jupyter Lab
Fixed the documentation for this to let users know the necessary installations for Jupyter Lab to work with plotly.