Skip to content

BUG: Fix bug in type conversion of arrays; add array and struct tests #101

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

Merged
merged 2 commits into from
Jan 2, 2018

Conversation

jasonqng
Copy link
Contributor

As per @tswast's requests in #22 and #23, tests added to check that arrays and structs are successfully returned and named properly from a query.

While running tests, identified a bug with the type conversion in read_gbq (see below). Updated it so it only performs type conversion with the added condition that the returned field mode IS NOT repeated.

To re-produce type conversion error with array:

>>> import gbq
>>> query = """WITH t as (
...         SELECT "a" letter, 1 num
...         UNION ALL
...         SELECT "b" letter, 2 num
...         UNION ALL
...         SELECT "a" letter, 3 num)
...
...         select letter, array_agg(num order by num ASC) num_array
...         from t
...         group by letter"""
>>> df = gbq.read_gbq(query, project_id='XXXXX',dialect='standard',verbose=False)

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "gbq.py", line 877, in read_gbq
    final_df[field['name']].astype(type_map[field['type'].upper()])
  File "/Users/jasonng/anaconda2/lib/python2.7/site-packages/pandas/util/_decorators.py", line 118, in wrapper
    return func(*args, **kwargs)
  File "/Users/jasonng/anaconda2/lib/python2.7/site-packages/pandas/core/generic.py", line 4004, in astype
    **kwargs)
  File "/Users/jasonng/anaconda2/lib/python2.7/site-packages/pandas/core/internals.py", line 3462, in astype
    return self.apply('astype', dtype=dtype, **kwargs)
  File "/Users/jasonng/anaconda2/lib/python2.7/site-packages/pandas/core/internals.py", line 3329, in apply
    applied = getattr(b, f)(**kwargs)
  File "/Users/jasonng/anaconda2/lib/python2.7/site-packages/pandas/core/internals.py", line 544, in astype
    **kwargs)
  File "/Users/jasonng/anaconda2/lib/python2.7/site-packages/pandas/core/internals.py", line 625, in _astype
    values = astype_nansafe(values.ravel(), dtype, copy=True)
  File "/Users/jasonng/anaconda2/lib/python2.7/site-packages/pandas/core/dtypes/cast.py", line 692, in astype_nansafe
    return lib.astype_intsafe(arr.ravel(), dtype).reshape(arr.shape)
  File "pandas/_libs/lib.pyx", line 854, in pandas._libs.lib.astype_intsafe
  File "pandas/_libs/src/util.pxd", line 91, in util.set_value_at_unsafe
ValueError: setting an array element with a sequence.

@codecov-io
Copy link

codecov-io commented Dec 30, 2017

Codecov Report

Merging #101 into master will decrease coverage by 44.57%.
The diff coverage is 25%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #101       +/-   ##
===========================================
- Coverage    73.5%   28.93%   -44.58%     
===========================================
  Files           4        4               
  Lines        1491     1507       +16     
===========================================
- Hits         1096      436      -660     
- Misses        395     1071      +676
Impacted Files Coverage Δ
pandas_gbq/gbq.py 20.55% <ø> (-55.94%) ⬇️
pandas_gbq/tests/test_gbq.py 27.92% <25%> (-54.42%) ⬇️

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 4cab83c...f459197. Read the comment docs.

Copy link
Collaborator

@tswast tswast left a comment

Choose a reason for hiding this comment

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

Thanks!

@tswast
Copy link
Collaborator

tswast commented Jan 2, 2018

@jasonqng Could you add this to the 0.3.0 entry in the changelog? https://github.com/pydata/pandas-gbq/blob/master/docs/source/changelog.rst

@jasonqng
Copy link
Contributor Author

jasonqng commented Jan 2, 2018

@tswast Added, feel free to tweak.

@tswast tswast merged commit 7ca44bd into googleapis:master Jan 2, 2018
@jasonqng jasonqng deleted the array_struct_tests branch April 6, 2018 03:38
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