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

[ENH] pull in warning for dialect change from pandas-gbq #22557

Merged
merged 1 commit into from Sep 18, 2018

Conversation

Projects
None yet
4 participants
@tswast
Copy link
Contributor

commented Aug 31, 2018

  • closes towards pydata/pandas-gbq#195
  • tests added / passed (N/A)
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

@tswast tswast changed the title [ENH] pull in warning for dialect change from pandas-gbq. [ENH] pull in warning for dialect change from pandas-gbq Aug 31, 2018

@codecov

This comment has been minimized.

Copy link

commented Aug 31, 2018

Codecov Report

Merging #22557 into master will decrease coverage by 0.13%.
The diff coverage is 25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #22557      +/-   ##
==========================================
- Coverage   92.17%   92.03%   -0.14%     
==========================================
  Files         169      169              
  Lines       50708    50791      +83     
==========================================
+ Hits        46740    46746       +6     
- Misses       3968     4045      +77
Flag Coverage Δ
#multiple 90.44% <25%> (-0.14%) ⬇️
#single 42.29% <25%> (-0.06%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 25% <25%> (ø) ⬆️
pandas/errors/__init__.py 92.3% <0%> (-7.7%) ⬇️
pandas/core/dtypes/base.py 92.68% <0%> (-7.32%) ⬇️
pandas/core/arrays/base.py 88% <0%> (-6.25%) ⬇️
pandas/io/html.py 89.17% <0%> (-2.08%) ⬇️
pandas/io/formats/html.py 88.81% <0%> (-1.87%) ⬇️
pandas/core/arrays/datetimelike.py 94.02% <0%> (-1.75%) ⬇️
pandas/core/arrays/datetimes.py 95.52% <0%> (-1.3%) ⬇️
pandas/io/parquet.py 71.79% <0%> (-1.25%) ⬇️
pandas/core/base.py 96.92% <0%> (-0.69%) ⬇️
... and 25 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 73dd6ec...0de8d22. Read the comment docs.

FutureWarning,
stacklevel=2,
)

This comment has been minimized.

Copy link
@gfyoung

gfyoung Sep 1, 2018

Member

Do we have a good way to test this by any chance?

This comment has been minimized.

Copy link
@tswast

tswast Sep 2, 2018

Author Contributor

I've got a test in the pandas-gbq repo:

https://github.com/pydata/pandas-gbq/blob/998b6cfe953613bf5ada6253a7d3550837a2d169/tests/unit/test_gbq.py#L367-L369

I suppose I should include a similar test here?

This comment has been minimized.

Copy link
@gfyoung

gfyoung Sep 2, 2018

Member

If it's possible to include in some shape or form, I would definitely be 👍 for it.

This comment has been minimized.

Copy link
@tswast

tswast Sep 4, 2018

Author Contributor

Added in 07c2eb8

mock_read_gbq = mock.Mock()
mock_read_gbq.return_value = DataFrame([[1.0]])
monkeypatch.setattr(pandas_gbq, 'read_gbq', mock_read_gbq)
with pytest.warns(FutureWarning):

This comment has been minimized.

Copy link
@jreback

jreback Sep 8, 2018

Contributor

use tm.assert_produces_warning, we don't use pytest.warns (and it fails our linter)

This comment has been minimized.

Copy link
@tswast

tswast Sep 11, 2018

Author Contributor

Done in 0de8d22

@@ -93,6 +98,14 @@ def make_mixed_dataframe_v2(test_size):
index=range(test_size))


def test_read_gbq_without_dialect_warns_future_change(monkeypatch):
mock_read_gbq = mock.Mock()

This comment has been minimized.

Copy link
@jreback

jreback Sep 8, 2018

Contributor

can you add the issue number as a comment here

This comment has been minimized.

Copy link
@tswast

tswast Sep 11, 2018

Author Contributor

Done in 0de8d22

@tswast tswast force-pushed the tswast:master branch from 0bb1832 to f5c07d2 Sep 11, 2018

@pep8speaks

This comment has been minimized.

Copy link

commented Sep 11, 2018

Hello @tswast! Thanks for updating the PR.

@tswast tswast force-pushed the tswast:master branch from f5c07d2 to 0de8d22 Sep 11, 2018

@tswast

This comment has been minimized.

Copy link
Contributor Author

commented Sep 13, 2018

Travis error appears to be a temporary issue with Conda downloads. https://travis-ci.org/pandas-dev/pandas/jobs/427334975

CondaHTTPError: HTTP 504 GATEWAY TIME-OUT for url https://conda.anaconda.org/conda-forge/linux-64/pycryptodome-3.6.6-py27h16a7912_0.tar.bz2

Maybe retrying it would help?

@jreback jreback added this to the 0.24.0 milestone Sep 15, 2018

@jreback
Copy link
Contributor

left a comment

tiny comment. ping on green.

@@ -65,6 +67,8 @@ def read_gbq(query, project_id=None, index_col=None, col_order=None,
*New in version 0.2.0 of pandas-gbq*.
dialect : str, default 'legacy'
Note: The default value is changing to 'standard' in a future verion.

This comment has been minimized.

Copy link
@jreback

jreback Sep 15, 2018

Contributor

can you add a versionchanged (0.24.0)

[ENH] pull in warning for dialect change from pandas-gbq.
* Add comment linking to pandas-gbq issue for change in default
  dialect.
* Add versionchanged to read_gbq dialect.

@tswast tswast force-pushed the tswast:master branch from 3289fd1 to cd18b7d Sep 17, 2018

@jreback jreback merged commit 1542d29 into pandas-dev:master Sep 18, 2018

6 checks passed

ci/circleci: py27_compat Your tests passed on CircleCI!
Details
ci/circleci: py35_ascii Your tests passed on CircleCI!
Details
ci/circleci: py36_locale Your tests passed on CircleCI!
Details
ci/circleci: py36_locale_slow Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jreback

This comment has been minimized.

Copy link
Contributor

commented Sep 18, 2018

thanks @tswast

aeltanawy pushed a commit to aeltanawy/pandas that referenced this pull request Sep 20, 2018

Sup3rGeo added a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018

@tswast tswast referenced this pull request Oct 24, 2018

Merged

TST: re-enable gbq tests #23303

2 of 2 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.