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

STY: use pytest.raises context syntax (series) #24812

Merged
merged 5 commits into from
Jan 17, 2019

Conversation

simonjayhawkins
Copy link
Member

xref #24332

@codecov
Copy link

codecov bot commented Jan 17, 2019

Codecov Report

Merging #24812 into master will decrease coverage by 49.45%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #24812       +/-   ##
===========================================
- Coverage   92.38%   42.93%   -49.46%     
===========================================
  Files         166      166               
  Lines       52382    52382               
===========================================
- Hits        48395    22488    -25907     
- Misses       3987    29894    +25907
Flag Coverage Δ
#multiple ?
#single 42.93% <ø> (ø) ⬆️
Impacted Files Coverage Δ
pandas/io/formats/latex.py 0% <0%> (-100%) ⬇️
pandas/core/categorical.py 0% <0%> (-100%) ⬇️
pandas/io/sas/sas_constants.py 0% <0%> (-100%) ⬇️
pandas/tseries/plotting.py 0% <0%> (-100%) ⬇️
pandas/tseries/converter.py 0% <0%> (-100%) ⬇️
pandas/io/formats/html.py 0% <0%> (-99.35%) ⬇️
pandas/core/groupby/categorical.py 0% <0%> (-95.46%) ⬇️
pandas/io/sas/sas7bdat.py 0% <0%> (-91.17%) ⬇️
pandas/io/sas/sas_xport.py 0% <0%> (-90.15%) ⬇️
pandas/core/tools/numeric.py 10.44% <0%> (-89.56%) ⬇️
... and 123 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 17a6bc5...ddc69b4. Read the comment docs.

@codecov
Copy link

codecov bot commented Jan 17, 2019

Codecov Report

Merging #24812 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #24812   +/-   ##
=======================================
  Coverage   92.38%   92.38%           
=======================================
  Files         166      166           
  Lines       52382    52382           
=======================================
  Hits        48395    48395           
  Misses       3987     3987
Flag Coverage Δ
#multiple 90.81% <ø> (ø) ⬆️
#single 42.92% <ø> (ø) ⬆️

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 17a6bc5...eae318a. Read the comment docs.

@jschendel jschendel added Testing pandas testing functions or related to the test suite Code Style Code style, linting, code_checks labels Jan 17, 2019
# with pytest.raises(TypeError, match=msg):
Series(Series(dates).astype('int') / 1000000, dtype='M8[ms]')
# Series(Series(dates).astype('int') / 1000000, dtype='M8[ms]')
pytest.raises(TypeError, lambda x: Series(
Copy link
Member Author

Choose a reason for hiding this comment

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

this is raising missing 1 required positional argument: 'x' because of the lambda and not failing as intended. correcting this with the conversion to context manger is only failing on windows and again from the error message is probably not failing as intended

Copy link
Contributor

Choose a reason for hiding this comment

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

this should be raising TypeError, what happens when you use the context manager

Copy link
Member Author

Choose a reason for hiding this comment

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

it is raising TypeError in master because of the lambda. this is a bad test.

correcting it with the context manager raises TypeError: cannot astype a datetimelike from [datetime64[ns]] to [int32] but only on the windows tests (see ci failures)

i've reverted the changes to the test, but the ci is failing on an unrelated conda error at the moment.

with the changes in this PR there will still be 823 uses of pytest.raises without the cm. so i don't want to get bogged down with bad tests at the moment.

Copy link
Contributor

Choose a reason for hiding this comment

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

k that's fine (though happy to take your change, though it should raise on windows as well). can you create an issue about this.

@jreback jreback added this to the 0.24.0 milestone Jan 17, 2019
@jreback jreback merged commit be26f6d into pandas-dev:master Jan 17, 2019
@jreback
Copy link
Contributor

jreback commented Jan 17, 2019

thanks!

msg = (r"The 'datetime64' dtype has no unit\. Please pass in"
r" 'datetime64\[ns\]' instead\.")
with pytest.raises(ValueError, match=msg):
Series(dates, dtype='datetime64')
Copy link
Member Author

Choose a reason for hiding this comment

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

i think this test is also not failing as intended and was masked by another lambda error. will do a follow-up PR to address theses cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Style Code style, linting, code_checks Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants