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

ARROW-3363: [C++/Python] Add helper functions to detect scalar Python types #2659

Closed
wants to merge 3 commits into from

Conversation

xhochy
Copy link
Member

@xhochy xhochy commented Sep 29, 2018

No description provided.

@xhochy
Copy link
Member Author

xhochy commented Sep 29, 2018

Required for #2623

@kszucs
Copy link
Member

kszucs commented Sep 29, 2018

@xhochy clang-format fails

@@ -1474,3 +1474,15 @@ def from_numpy_dtype(object dtype):
check_status(NumPyDtypeToArrow(dtype, &c_type))

return pyarrow_wrap_data_type(c_type)


def is_boolean_object(object obj):
Copy link
Member

@kszucs kszucs Sep 29, 2018

Choose a reason for hiding this comment

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

The naming and/or placement in types module feels a bit odd.

pa.types.is_integer_object(pa.types.int16())  # False

Perhaps is_<type>_value?

OR

How about creating a pyarrow.inference module with is_bool, is_int, is_float functions?

Copy link
Member

Choose a reason for hiding this comment

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

I like the _value name ok

@xhochy
Copy link
Member Author

xhochy commented Sep 30, 2018

@xhochy clang-format fails

Once #2572 is merged, I'll add a docker-compose run clang-format. It's really annoying that this continuously breaks for me when brew updates clang-format.

Copy link
Member

@kszucs kszucs left a comment

Choose a reason for hiding this comment

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

LGTM

@kszucs
Copy link
Member

kszucs commented Sep 30, 2018

Sounds good, to run docker-compose run clang-format arrow must be attached as a volume though.

@kszucs
Copy link
Member

kszucs commented Sep 30, 2018

CI failure is related to wheel incompatibilities.

@kszucs kszucs closed this in 7a4b48c Sep 30, 2018
@xhochy xhochy deleted the ARROW-3363 branch September 30, 2018 20:29
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.

3 participants