-
Notifications
You must be signed in to change notification settings - Fork 245
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
Basic python extension types for image, point2d, and box2d #102
Conversation
c55e7cb
to
cf82098
Compare
cf82098
to
98e9a35
Compare
python/lance/tests/test_types.py
Outdated
from lance.types import * | ||
|
||
|
||
@pytest.mark.skipif( |
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.
we can skip the entire file
@@ -0,0 +1,56 @@ | |||
import platform |
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.
License and isort
python/lance/types.py
Outdated
__all__ = ["ImageType", "ImageUriType", "ImageBinaryType", "Point2dType", "Box2dType"] | ||
|
||
|
||
# Arrow extension type |
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.
move imports above __all__ = [...]
?
Seems need to run isort as well.
import pyarrow as pa | ||
|
||
|
||
__all__ = ["ImageType", "ImageUriType", "ImageBinaryType", "Point2dType", "Box2dType"] |
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.
Should we not expose ImageType
, which is the base class?
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.
ImageType.from_storage allows the user to get the correct subtype directly from storage array so they don't need to think about whether they need a uri or binary type
pass | ||
|
||
def __arrow_ext_serialize__(self): | ||
return b"" |
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.
Curious what does this function do?
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.
For parametrized types this returns extra information (from the examples in Arrow docs)
This depends on #100 . The unit tests were segfaulting until I rebuilt liblance using #100 branch.