Skip to content

ARROW-1004: [Python] Add conversions for numpy object arrays with integers and floats #681

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.

Sign up for GitHub

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

Jump to bottom
Closed
wesm wants to merge 3 commits into from
Closed

ARROW-1004: [Python] Add conversions for numpy object arrays with integers and floats #681

wesm wants to merge 3 commits into from

Conversation

@wesm
Copy link
Member

@wesm wesm commented May 14, 2017

No description provided.

xhochy
xhochy requested changes May 14, 2017
View reviewed changes
cpp/src/arrow/python/pandas_convert.cc Outdated
Copy link
Member

@xhochy xhochy May 14, 2017

Choose a reason for hiding this comment

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

PyArray_IsIntegerScalar uses PyInt_Check with Python 3.4 and numpy==1.9.x Thus we probably need to raise our minimal NumPy version to >=1.10. For the manylinux1 build, you have to adjust it in python/manylinux1/scripts/build_virtualenvs.sh

Copy link
Member Author

@wesm wesm May 14, 2017

Choose a reason for hiding this comment

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

done

xhochy
xhochy reviewed May 14, 2017
View reviewed changes
python/manylinux1/scripts/build_virtualenvs.sh Outdated
Copy link
Member

@xhochy xhochy May 14, 2017

Choose a reason for hiding this comment

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

Please pin exactly here, we need the most minimal NumPy version at build time to stay compatible with as many numpy versions as possible.

@xhochy
Copy link
Member

xhochy commented May 14, 2017

I've made #684 to get a new base docker-image.

cpcloud
cpcloud reviewed May 14, 2017
View reviewed changes
cpp/src/arrow/python/pandas_convert.cc Outdated
Copy link
Contributor

@cpcloud cpcloud May 14, 2017

Choose a reason for hiding this comment

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

Any reason not to use RETURN_IF_PYERROR() here?

Copy link
Member Author

@wesm wesm May 14, 2017

Choose a reason for hiding this comment

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

nope, changed

cpcloud
cpcloud reviewed May 14, 2017
View reviewed changes
cpp/src/arrow/python/pandas_convert.cc Outdated
Copy link
Contributor

@cpcloud cpcloud May 14, 2017

Choose a reason for hiding this comment

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

Should this be PyFloat_AsDouble followed by RETURN_IF_PYERROR() to make sure that errors raised by Python are propagated?

Copy link
Member Author

@wesm wesm May 14, 2017

Choose a reason for hiding this comment

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

reasons why PyFloat_AsDouble would fail after passing PyFloat_Check seem esoteric to me, but I've changed it to be on the safe side

wesm added 3 commits May 14, 2017 15:38
@wesm
Add conversions for numpy object arrays with integers and floats
4e4c752 
Change-Id: I69c2960b510fd5e1eeef66ac614b40019b545825
@wesm
Fixes for manylinux1
45f1ecb 
Change-Id: Id1f5aa57b6fd090dc295a956e2a91b74e333fd96
@wesm
Code review comments
9e0b2ea 
Change-Id: Icaf053db0b8141af18fe19a8e11e9541cc591af9
cpcloud
cpcloud approved these changes May 14, 2017
View reviewed changes
Copy link
Contributor

@cpcloud cpcloud left a comment

Choose a reason for hiding this comment

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

This looks good to me.

@wesm
Copy link
Member Author

wesm commented May 14, 2017

Cool, here is the appveyor build: https://ci.appveyor.com/project/wesm/arrow/build/1.0.431

@asfgit asfgit closed this in 37dbddf May 14, 2017
jeffknupp pushed a commit to jeffknupp/arrow that referenced this pull request Jun 3, 2017
@wesm @jeffknupp
ARROW-1004: [Python] Add conversions for numpy object arrays with int…
45521f2 
…egers and floats

Author: Wes McKinney <wes.mckinney@twosigma.com>

Closes apache#681 from wesm/ARROW-1004 and squashes the following commits:

9e0b2ea [Wes McKinney] Code review comments
45f1ecb [Wes McKinney] Fixes for manylinux1
4e4c752 [Wes McKinney] Add conversions for numpy object arrays with integers and floats
pribor pushed a commit to GlobalWebIndex/arrow that referenced this pull request Oct 24, 2025
@jbonofre
MINOR: Upgrade to Apache POM 34 (apache#681)
ebe63ff 
This PR updates to parent Apache POM 34 with new plugins versions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xhochy xhochy xhochy requested changes

@cpcloud cpcloud cpcloud approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@wesm @xhochy @cpcloud