-
Notifications
You must be signed in to change notification settings - Fork 92
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
Adds plugin, example and tests for Vaex (#457) #708
Conversation
examples/vaex/my_script.py
Outdated
import sys | ||
|
||
from hamilton import base, driver | ||
from hamilton.plugins import h_vaex, vaex_extensions # noqa F401 |
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.
@tyapochkin we can drop vaex_extensions
here and instead add vaex
to https://github.com/DAGWorks-Inc/hamilton/blob/main/hamilton/function_modifiers/base.py#L23
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.
looking good just please add vaex
to the plugins list in function_modifiers/base.py -- that'll save people an import :)
I'll then run this locally this week, and then we can plan to release this next week. 🚀
Great job on the PR! Excited to try it! @skrawcz @elijahbenizzy Should we move tests to We have a growing list of test dependencies for plugins / extensions and we might make that more modular in CI tasks? |
Sorry @tyapochkin @zilto I confused myself with what I was talking about earlier. I think this test setup is fine, and mirrors what we have for spark, etc. Basically, if it's special then |
Adding to the registry so we try to load the extension if vaex is in the environment. Then fixing a few minor typos otherwise to get things to work.
Yeah, my logic is the following:
So for me, separating tests for vaex in distinct folder outside |
yep yep, I think what you have is fine. |
Adds minor fixes for Vaex
This PR adds a plugin for Vaex and closes #457
Changes
hamilton/plugins/vaex_extensions.py
: registration of vaex typeshamilton/plugins/h_vaex.py
: code of VaexDataFrameResultplugin_tests/h_vaex/test_h_vaex.py
: unit testsplugin_tests/h_vaex/resources/functions.py
: functions for unit testsexamples/vaex/notebook.ipynb
: example with vaex as a jupyter notebookexamples/vaex/my_script.py
: example with vaex as a scriptexamples/vaex/my_functions.py
: functions for example with vaexexamples/vaex/requirements.txt
: requirements for example with vaex.circleci/config.yml
: added new task for vaex.ci/setup.sh
: added new task for vaex.ci/test.sh
: added new task for vaexsetup.py
: added new installation optionsf-hamilton[vaex]
How I tested this
I added and ran a new CircleCI job with unit tests for VaexDataFrameResult.
Notes
Vaex builds its wheels so long and depends on so many additional packages, that I decided to take it out in the separate installation option
sf-hamilton[vaex]
and separate CircleCI taskvaex-py39
. Right now, it doesn't support Python 3.11 and 3.12, so I created only one task for just one Python version 3.9.Checklist