Skip to content
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

Merged
merged 8 commits into from
Aug 24, 2022

Conversation

changhiskhan
Copy link
Contributor

This depends on #100 . The unit tests were segfaulting until I rebuilt liblance using #100 branch.

@changhiskhan changhiskhan requested a review from eddyxu August 14, 2022 07:32
@changhiskhan changhiskhan force-pushed the changhiskhan/extension-types branch 2 times, most recently from c55e7cb to cf82098 Compare August 16, 2022 22:26
@changhiskhan changhiskhan force-pushed the changhiskhan/extension-types branch from cf82098 to 98e9a35 Compare August 24, 2022 00:17
@changhiskhan changhiskhan marked this pull request as ready for review August 24, 2022 01:09
from lance.types import *


@pytest.mark.skipif(
Copy link
Contributor

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

https://stackoverflow.com/a/42512169

@@ -0,0 +1,56 @@
import platform
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

License and isort

__all__ = ["ImageType", "ImageUriType", "ImageBinaryType", "Point2dType", "Box2dType"]


# Arrow extension type
Copy link
Contributor

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"]
Copy link
Contributor

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?

Copy link
Contributor Author

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""
Copy link
Contributor

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?

Copy link
Contributor Author

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)

@changhiskhan changhiskhan merged commit c60b517 into main Aug 24, 2022
@changhiskhan changhiskhan deleted the changhiskhan/extension-types branch August 24, 2022 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants