Added query_config parameter to read_gbq
State: closed

Now more complicated queries could be processed.

Labels:

Comments:
cc @parthea 

accepting kwargs is a more general way of doing this. I don't really want to have to keep adding specific keywords.

Futher would need some tests.
@jreback thanks for your notes. I've changed parameters to kwargs style.
Could you give me some examples how to make tests and publish it as you mentioned? I've done it on my local but I think it is not enough.
https://github.com/pandas-dev/pandas/blob/master/pandas/io/tests/test_gbq.py

for tests and http://pandas-docs.github.io/pandas-docs-travis/contributing.html#running-google-bigquery-integration-tests
## [Current coverage](https://codecov.io/gh/pandas-dev/pandas/pull/14742?src=pr) is 84.75% (diff: 7.69%)
> Merging [#14742](https://codecov.io/gh/pandas-dev/pandas/pull/14742?src=pr) into [master](https://codecov.io/gh/pandas-dev/pandas/branch/master?src=pr) will decrease coverage by **0.02%**

```diff
@@             master     #14742   diff @@
==========================================
  Files           145        145          
  Lines         51090      51139    +49   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43315      43344    +29   
- Misses         7775       7795    +20   
  Partials          0          0          
```

> Powered by [Codecov](https://codecov.io?src=pr). Last update [e27b296...3a238a5](https://codecov.io/gh/pandas-dev/pandas/compare/e27b29697f0dcf9359f01a19edb2f20c6d728b6c...3a238a540d7e6843ecb52df8f157d6e3125117f5?src=pr)
@jreback do I must write some additional test in https://github.com/pandas-dev/pandas/blob/master/pandas/io/tests/test_gbq.py? As I see current version is passed by bot successfully.
If yes, do I need just write test and push it, or I must make these manipulations http://pandas-docs.github.io/pandas-docs-travis/contributing.html#running-google-bigquery-integration-tests before?
> do I must write some additional test in https://github.com/pandas-dev/pandas/blob/master/pandas/io/tests/test_gbq.py? As I see current version is passed by bot successfully.

Correct. There should be new unit test(s) added in `test_gbq.py`

>If yes, do I need just write test and push it, or I must make these manipulations http://pandas-docs.github.io/pandas-docs-travis/contributing.html#running-google-bigquery-integration-tests before?

Please follow the steps in the link mentioned. Ideally this is done before you push. You only need to do this once and hopefully you won't touch it again, except to change credentials if needed. It should only take 5-10 minutes. Separately, please let me know if any part of the instructions is unclear (I put the instructions together). 
@parthea thank you for your notes.
I've added 2 new tests based on query config. 
I'm not sure about ```test_query_with_parameters``` because  as I see ```parameters``` is an experimental feature now.

BQ testing instruction is quite good, although I have problems with credential and my build crashes on my own Travis-CI tests. I've got an exception 
```
InvalidPrivateKeyFormat: Private key is missing or invalid. It should be service account private key JSON (file path or string contents) with at least two keys: 'client_email' and 'private_key'
```
although I've put my JSON in single quotes

So, is it enough for my pull-request?
@necnec sorry to ask you to move it again, but as this is not a critical fix or enhancement, let's keep this for 0.20, so can you move the whatsnew notice to v0.20.0.txt?
I think it would be good to add some documentation about this in the tutorial docs? (http://pandas.pydata.org/pandas-docs/stable/io.html#google-bigquery-experimental)
you can just use a passed in config dict to update the query

this should be like 2 lines of code for the change
lgtm. 


@parthea ok?
@necnec 

> @jorisvandenbossche about kwargs instead of configuration you can find there: #14742 (comment)
I think @jreback has better explanation why we use kwargs not configuration


kwargs can be used to pass through a bunch of keywords, but this is a single specific keyword that pandas implements to hold those kwargs, so I think we should document it like that.


Further, the user can now pass other keys in the `config` object besides 'query', and those will be ignored but no error/warning is raised?
@jorisvandenbossche 
> kwargs can be used to pass through a bunch of keywords, but this is a single specific keyword that pandas implements to hold those kwargs, so I think we should document it like that.

Sorry, I haven't got your idea. Do you think we should use ```config``` argument instead of ```kwargs``` ? It was my first version but @jreback reasonably ask me to change it to ```kwargs```. I'm ready to add some more documentation if you show me where.

>Further, the user can now pass other keys in the config object besides 'query', and those will be ignored but no error/warning is raised?

```config``` allows only 'query' options. Otherwise, it throws an exception. Unit test named ```test_configuration_without_query``` check that
> config allows only 'query' options. Otherwise, it throws an exception. Unit test named test_configuration_without_query check that

Yes, but if you pass a config object with 'query' and such a 'copy' key, the 'copy' key will be silently ignored?
@jorisvandenbossche Yes it will be ignored. Do you think it should throws an exception in this case?
@necnec IMO, yes, I would raise an error, that is more informative to the user (or at least raise a warning)
@jorisvandenbossche I've added exception if you pass 2 types of job in config. But I haven't added additional unit test.
So after test this feature on my local I receive exception something like this:
```ValueError: Only one job type must be specified, but given query,load```
I'm not sure that message looks well so I'll happy if you help me to make it more clear.
@necnec the reason I thought it a nice idea to allow arbitrary kwargs to be passed is that I don't want pandas having to add lots of kwargs to the signature when they are simply passed thru.

does this assertion still hold? is this useful?
Yes, I think adding kwargs is a good idea to pass arbitrary keys
@necnec just a couple of doc comments. ping when green.
@jreback Repeating my question from above: I know you asked to rename the kwarg from configuration to config, but as this is to specifically pass the 'configuration' from GBQ terminology, shouldn't we rather keep it consistent with their naming and use 'configuration' as well?
I just though ``config`` is more in-line with what is actually used in the JSON (and a bit shorter). If we are actually using ``configuration`` (to pass the data) then that would be fine too.
@jreback yes, the key in the dict / json that is passed is actually `'configuration'`, therefore I would use that.
@necnec ok pls make the adjustment from ``config`` -> ``configuration`` as @jorisvandenbossche indicates. ping on green.
thanks @necnec 

sometimes these things takes time. thanks for the patience.
Thank you too for this experience! I'm glad to be participated in it.