-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Remove numpy dependency for Infogram #7141
Comments
Erin LeDell commented: [~accountid:5e43370f5a495e0c91a74ebe] Can you check that my proposal makes sense? Or is there a better way to make numpy available in the {{plot()}} method? Thx. |
Tomas Fryda commented: [~accountid:557058:afd6e9a4-1891-4845-98ea-b5d34a2bc42c] There is a function called {{_get_numpy}}. So it should be possible just to move that function somewhere like {{h2o.utils.ext_dependencies}}. (I think that’s where we had {{get_matplotlib}} before refactoring). [https://github.com/h2oai/h2o-3/blob/4734a2ce9dcf4886423147ff51454fa49fad5dde/h2o-py/h2o/model/model_base.py#L1785-L1793|https://github.com/h2oai/h2o-3/blob/4734a2ce9dcf4886423147ff51454fa49fad5dde/h2o-py/h2o/model/model_base.py#L1785-L1793|smart-link] |
Erin LeDell commented: I tested this in a venv w/o numpy installed. You will actually hit the matplotlib depedency first, so the error raised is: {noformat}>>> ig.plot() |
JIRA Issue Details Jira Issue: PUBDEV-8521 |
Linked PRs from JIRA |
We should not be importing numpy by default. It's a dependency for the Infogram plot() method in Python, but we need to keep it isolated there.
Remove this line: [https://github.com/h2oai/h2o-3/blob/master/h2o-bindings/bin/custom/python/gen_infogram.py#L241|https://github.com/h2oai/h2o-3/blob/master/h2o-bindings/bin/custom/python/gen_infogram.py#L241]
Even though numpy is a dependency of matplotlib, we should still directly import it somewhere so that the plot() method can use it: [https://github.com/h2oai/h2o-3/blob/master/h2o-bindings/bin/custom/python/gen_infogram.py#L76|https://github.com/h2oai/h2o-3/blob/master/h2o-bindings/bin/custom/python/gen_infogram.py#L76]
Maybe we just make a {{get_numpy}} function similar to what I did for {{get_pollycollection}}? [https://github.com/h2oai/h2o-3/blob/046a0a2438b01926eca936d278cf5b9569200589/h2o-py/h2o/plot/init.py#L2|https://github.com/h2oai/h2o-3/blob/046a0a2438b01926eca936d278cf5b9569200589/h2o-py/h2o/plot/init.py#L2|smart-link] by adding a {{._numpy}} file: [https://github.com/h2oai/h2o-3/blob/046a0a2438b01926eca936d278cf5b9569200589/h2o-py/h2o/plot/_polycollection.py|https://github.com/h2oai/h2o-3/blob/046a0a2438b01926eca936d278cf5b9569200589/h2o-py/h2o/plot/_polycollection.py|smart-link] and then import it here: [https://github.com/h2oai/h2o-3/blob/master/h2o-bindings/bin/custom/python/gen_infogram.py#L44|https://github.com/h2oai/h2o-3/blob/master/h2o-bindings/bin/custom/python/gen_infogram.py#L44|smart-link]
Test this in a venv with numpy not installed.
The text was updated successfully, but these errors were encountered: