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

Add execute_sql function to enable execution of SQL code where no DataFrame is returned #90

Closed
wants to merge 1 commit into from

Conversation

madhav-datt
Copy link

Initial pull request based on the discussion and issue explained here. I'd love to discuss any changes to the design of this function, or if it might be better to add this functionality elsewhere.

@codecov-io
Copy link

codecov-io commented Oct 14, 2017

Codecov Report

Merging #90 into master will decrease coverage by 45.46%.
The diff coverage is 20%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #90       +/-   ##
===========================================
- Coverage   73.44%   27.98%   -45.47%     
===========================================
  Files           4        4               
  Lines        1578     1583        +5     
===========================================
- Hits         1159      443      -716     
- Misses        419     1140      +721
Impacted Files Coverage Δ
pandas_gbq/gbq.py 19.35% <20%> (-56.96%) ⬇️
pandas_gbq/tests/test_gbq.py 27.76% <0%> (-54.18%) ⬇️

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 9eb9d77...73a8593. Read the comment docs.

private_key=private_key,
auth_local_webserver=auth_local_webserver,
dialect=dialect, **kwargs)
except KeyError:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's mostly coincidental that the existing function raises a KeyError. Can we find a more coherent way of doing this?

@tswast
Copy link
Collaborator

tswast commented Jan 3, 2018

Could you rebase this PR on the latest version in master? There have been some significant changes since this was opened. We changed what library we use to actually make the API calls to BigQuery.

@madhav-datt
Copy link
Author

@tswast Thanks so much for the update. I'll change this PR based on the latest code in master.

@max-sixty
Copy link
Contributor

@madhav-datt Would love to merge this. Let us know if you're up for updating this.

@tswast
Copy link
Collaborator

tswast commented Apr 3, 2018

I've been thinking about this in regards to dryRun queries, too. (#88) I think it might make some sense for read_gbq to return an empty dataframe in the case when there are no result rows.

@madhav-datt
Copy link
Author

Thanks for the follow up! Sure - I'll go ahead and update the pull request.

@max-sixty
Copy link
Contributor

@madhav-datt this is v close! How would you feel about pushing over the line?

@tswast
Copy link
Collaborator

tswast commented Aug 9, 2019

Thank you for the contribution @madhav-datt and your patience. I'm closing this in favor of #286 which adds a max_results argument to read_gbq. Set max_results=0 if no results are desired.

@tswast tswast closed this Aug 9, 2019
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.

4 participants