-
Notifications
You must be signed in to change notification settings - Fork 10
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
Fix issue #467 - Move imports to rbc.heavydb #470
Fix issue #467 - Move imports to rbc.heavydb #470
Conversation
202c45f
to
d884525
Compare
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.
Nice! I went over all the files, LGTM. Just 2 remarks.
Ultimately, why don't we just move heavydb
into heavydb_backend
?
c9da34e
to
5335a1f
Compare
5335a1f
to
8056911
Compare
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.
LGTM now. Although I see you adding files again. Are you missing more?
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.
LGTM!
Re @tupui comment: Ultimately, why don't we just move heavydb into heavydb_backend?
Yes, I think it makes sense but as a follow-up PR:
- move
heavydb.py
toheavydb_backend/remote_heavydb.py
(or suggest a better name) - rename
heavydb_backend/
toheavydb/
- expose the UI of
heavydb.py
viaheavydb/__init__.py
I've messed up during rebase 😆 |
As title.
rbc.heavai.*
torbc.heavydb.*
rbc/tests/heavyai
torbc/tests/heavydb
rbc/tests/heavydb/test_imports.py
that ensures imports forArray
,ArrayPointer
,Bytes
,Column
are working as expected. Some of those imports are used in UD[T]Fs to create objects at runtime, others are used internally in jitted functions.