Skip to content

ARROW-2694 - [Python] ArrayValue string conversion returns the representation instead of the converted python object string#2130

Closed
fjetter wants to merge 1 commit intoapache:masterfrom
fjetter:bugfix/scalar_string_conversion
Closed

ARROW-2694 - [Python] ArrayValue string conversion returns the representation instead of the converted python object string#2130
fjetter wants to merge 1 commit intoapache:masterfrom
fjetter:bugfix/scalar_string_conversion

Conversation

@fjetter
Copy link
Contributor

@fjetter fjetter commented Jun 10, 2018

Example:

# python 3.6.5
In [1]: import pyarrow as pa

In [2]: str(pa.array(['a'])[0])  # note the single quotes
Out[2]: "'a'"

In [3]: str(pa.array([1], pa.timestamp('s'))[0])
Out[3]: "Timestamp('1970-01-01 00:00:01')"

instead of

# python 3.6.5
In [1]: import pyarrow as pa

In [2]: str(pa.array(['a'])[0])
Out[2]: "a"

In [3]: str(pa.array([1], pa.timestamp('s'))[0])
Out[3]: "1970-01-01 00:00:01"

@fjetter fjetter force-pushed the bugfix/scalar_string_conversion branch from b5893d5 to 2ead142 Compare June 10, 2018 19:58
@codecov-io
Copy link

codecov-io commented Jun 10, 2018 •
edited
Loading

Codecov Report

Merging #2130 into master will decrease coverage by 0.02%.
The diff coverage is 90.9%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2130      +/-   ##
==========================================
- Coverage   86.39%   86.37%   -0.03%     
==========================================
  Files         242      230      -12     
  Lines       41475    40599     -876     
==========================================
- Hits        35832    35067     -765     
+ Misses       5643     5532     -111
Impacted Files Coverage Δ
python/pyarrow/tests/test_scalars.py 100% <100%> (ø) ⬆️
python/pyarrow/tests/test_convert_builtin.py 99.6% <100%> (ø) ⬆️
python/pyarrow/scalar.pxi 61.57% <50%> (-0.2%) ⬇️
rust/src/memory_pool.rs
rust/src/buffer.rs
rust/src/error.rs
rust/src/builder.rs
rust/src/datatypes.rs
rust/src/lib.rs
rust/src/list.rs
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 34890cc...991b49e. Read the comment docs.

wesm
wesm approved these changes Jun 10, 2018
View reviewed changes
Copy link
Member

@wesm wesm left a comment

Choose a reason for hiding this comment

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

+1, except for the _check_null question, I will pull down the branch to take a look

python/pyarrow/scalar.pxi Outdated
Copy link
Member

@wesm wesm Jun 10, 2018

Choose a reason for hiding this comment

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

I believe _check_null is no longer needed, @kszucs is that right?

Copy link
Member

@kszucs kszucs Jun 11, 2018

Choose a reason for hiding this comment

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

@wesm after merging #2132 _check_null won't be needed

@wesm wesm force-pushed the bugfix/scalar_string_conversion branch from 2ead142 to bdd4cf1 Compare June 11, 2018 20:57
@wesm wesm force-pushed the bugfix/scalar_string_conversion branch from bdd4cf1 to 991b49e Compare June 11, 2018 21:35
xhochy
xhochy approved these changes Jun 14, 2018
View reviewed changes
Copy link
Member

@xhochy xhochy left a comment

Choose a reason for hiding this comment

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

+1, LGTM

@xhochy xhochy closed this in df44691 Jun 14, 2018
@fjetter fjetter deleted the bugfix/scalar_string_conversion branch June 14, 2018 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kszucs kszucs kszucs left review comments

@xhochy xhochy xhochy approved these changes

@wesm wesm wesm 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.

5 participants

@fjetter @codecov-io @xhochy @wesm @kszucs