STY: use pytest.raises context syntax (series/indexing/*) #24750

simonjayhawkins · 2019-01-13T13:01:25Z

@pytest.mark.xfail applied to test_where_unsafe_itemsize_fail : does not raise using python index operator instead of __setitem__ with tuple

pandas/tests/series/indexing/test_datetime.py

simonjayhawkins · 2019-01-13T13:08:19Z

pandas/tests/series/indexing/test_boolean.py

@@ -175,7 +183,8 @@ def test_where_unsafe_itemsize_fail(dtype):
    mask = s < 5

    values = [2.5, 3.5, 4.5, 5.5, 6.5]
-    pytest.raises(Exception, s.__setitem__, tuple(mask), values)
+    with pytest.raises(Exception):
+        s[mask] = values


does not raise using python index operator instead of setitem with tuple

not clear what you mean here?

@jreback

pandas/pandas/tests/series/indexing/test_boolean.py

Lines 163 to 187 in cc9fdef

@pytest.mark.parametrize("dtype", [np.int64, np.float64])

def test_where_unsafe_upcast(dtype):

s = Series(np.arange(10), dtype=dtype)

values = [2.5, 3.5, 4.5, 5.5, 6.5]

mask = s < 5

expected = Series(values + lrange(5, 10), dtype="float64")

s[mask] = values

assert_series_equal(s, expected)

@pytest.mark.xfail

@pytest.mark.parametrize("dtype", [

np.int8, np.int16, np.int32, np.float32

])

def test_where_unsafe_itemsize_fail(dtype):

# Can't do these, as we are forced to change the

# item size of the input to something we cannot.

s = Series(np.arange(10), dtype=dtype)

mask = s < 5

values = [2.5, 3.5, 4.5, 5.5, 6.5]

with pytest.raises(Exception):

s[mask] = values

looking at test above test_where_unsafe_upcast i'm assuming that it should be

s[mask] = values

whereas the current test, tests

s[tuple(mask)] = values

which gives KeyError: "None of [Index([True, True, True, True, True, False, False, False, False, False], dtype='object')] are in the [index]"

which does not appear to be the intended tested behaviour whereas the test is not raising an exception with the change.

can you match the expection more specifically with an error message, would make it more informative

test originally added in #3292 so i'm guessing that the expected error message was originally:

https://github.com/jreback/pandas/blob/b379772359a06d2fe9a646ad60bc7c4c125bfd6a/pandas/core/common.py#L780-L783

maybe fixed by #9743. this is where the error message was removed from core/common.py

codecov · 2019-01-13T14:33:12Z

Codecov Report

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

@@             Coverage Diff             @@
##           master   #24750       +/-   ##
===========================================
- Coverage   92.38%   43.07%   -49.32%     
===========================================
  Files         166      166               
  Lines       52358    52358               
===========================================
- Hits        48373    22551    -25822     
- Misses       3985    29807    +25822

Flag	Coverage Δ
#multiple	`?`
#single	`43.07% <ø> (-0.01%)`	⬇️

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 33f91d8...f6e1158. Read the comment docs.

codecov · 2019-01-13T14:33:12Z

Codecov Report

Merging #24750 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #24750      +/-   ##
==========================================
- Coverage   92.38%   92.38%   -0.01%     
==========================================
  Files         166      166              
  Lines       52358    52363       +5     
==========================================
+ Hits        48373    48377       +4     
- Misses       3985     3986       +1

Flag	Coverage Δ
#multiple	`90.81% <ø> (-0.01%)`	⬇️
#single	`42.91% <ø> (-0.17%)`	⬇️

Impacted Files	Coverage Δ
pandas/io/formats/console.py	`72.46% <0%> (-1.78%)`	⬇️
pandas/tseries/offsets.py	`96.69% <0%> (ø)`	⬆️
pandas/core/dtypes/dtypes.py	`95.6% <0%> (+0.02%)`	⬆️
pandas/util/testing.py	`88.09% <0%> (+0.09%)`	⬆️

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 33f91d8...abfe72d. Read the comment docs.

jreback · 2019-01-13T18:55:21Z

pandas/tests/series/indexing/test_boolean.py

@@ -165,6 +172,7 @@ def test_where_unsafe_upcast(dtype):
    assert_series_equal(s, expected)


+@pytest.mark.xfail


why does this fail? can you add a reason

jreback · 2019-01-13T18:56:14Z

pandas/tests/series/indexing/test_boolean.py

@@ -175,7 +183,8 @@ def test_where_unsafe_itemsize_fail(dtype):
    mask = s < 5

    values = [2.5, 3.5, 4.5, 5.5, 6.5]
-    pytest.raises(Exception, s.__setitem__, tuple(mask), values)
+    with pytest.raises(Exception):
+        s[mask] = values


not clear what you mean here?

jreback · 2019-01-13T19:25:47Z

pandas/tests/series/indexing/test_boolean.py

@@ -175,7 +183,8 @@ def test_where_unsafe_itemsize_fail(dtype):
    mask = s < 5

    values = [2.5, 3.5, 4.5, 5.5, 6.5]
-    pytest.raises(Exception, s.__setitem__, tuple(mask), values)
+    with pytest.raises(Exception):
+        s[mask] = values


can you match the expection more specifically with an error message, would make it more informative

jreback · 2019-01-13T21:21:13Z

pandas/tests/series/indexing/test_boolean.py

    s = Series(np.arange(10), dtype=dtype)
    mask = s < 5

    values = [2.5, 3.5, 4.5, 5.5, 6.5]
-    pytest.raises(Exception, s.__setitem__, tuple(mask), values)
+    s[mask] = values


so this should still raise if you try to set with a tuple though

a tuple will give a ValueError: Can only tuple-index with a MultiIndex which is tested in series/test_indexing.py,

the tuple keyword will create a tuple from an iterator, in this case a boolean series.

>>> import numpy as np >>> dtype = np.int8 >>> from pandas import Series >>> s = Series(np.arange(10), dtype=dtype) >>> mask = s < 5 >>> print(mask) 0 True 1 True 2 True 3 True 4 True 5 False 6 False 7 False 8 False 9 False dtype: bool >>> tuple(mask) (True, True, True, True, True, False, False, False, False, False) >>> values = [2.5, 3.5, 4.5, 5.5, 6.5] >>> s[tuple(mask)] = values

raises

KeyError: "None of [Index([True, True, True, True, True, False, False, False, False, False], dtype='object')] are in the [index]"

should we test for this?

I think the intention was to pass a tuple of values (not tupleify one), e.g.
s[tuple(mask, )] = values

in order to keep this PR more atomic should i revert the changes to this test and raise an separate issue?

you can if you want or just fix it here, should be straightforward

@jreback

should have said, ci failure is unrelated.. ##[error]doc/source/comparison_with_sas.rst(754,-22): error F821: undefined name 'get_ipython'

yes #24756 will address that

if the original intention was to raise a Exception: cannot change dtype of input to smaller size and that is no longer a restriction then the test is now fixed

what tests catches this?

@jreback

what tests catches this?

did you see my comments re #3292 and #9743

ok i c, so this was in essence an old test.

jreback · 2019-01-13T23:57:22Z

thanks @simonjayhawkins

…#24750)

STY: use pytest.raises context syntax

cc9fdef

simonjayhawkins commented Jan 13, 2019

View reviewed changes

pandas/tests/series/indexing/test_datetime.py Outdated Show resolved Hide resolved

simonjayhawkins commented Jan 13, 2019

View reviewed changes

simonjayhawkins added 2 commits January 13, 2019 14:12

use python index operator

6b4184b

fix ci errors

f6e1158

jreback requested changes Jan 13, 2019

View reviewed changes

jreback added the Code Style Code style, linting, code_checks label Jan 13, 2019

add xfail reason

c648737

jreback requested changes Jan 13, 2019

View reviewed changes

change test_where_unsafe_itemsize_fail to test_where_upcast

abfe72d

jreback requested changes Jan 13, 2019

View reviewed changes

jreback approved these changes Jan 13, 2019

View reviewed changes

jreback added this to the 0.24.0 milestone Jan 13, 2019

jreback merged commit 306e733 into pandas-dev:master Jan 13, 2019

simonjayhawkins deleted the cm-series-indexing branch January 15, 2019 19:54

simonjayhawkins added a commit to simonjayhawkins/pandas that referenced this pull request Jan 16, 2019

TST/CLN: follow-up to pandas-dev#24750

f8e5af0

simonjayhawkins mentioned this pull request Jan 16, 2019

TST/CLN: follow-up to #24750 #24810

Merged

jreback pushed a commit that referenced this pull request Jan 17, 2019

TST/CLN: follow-up to #24750 (#24810)

9f4357b

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

STY: use pytest.raises context syntax (series/indexing/*) (pandas-dev…

d6e48f9

…#24750)

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

TST/CLN: follow-up to pandas-dev#24750 (pandas-dev#24810)

91ba263

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

STY: use pytest.raises context syntax (series/indexing/*) (pandas-dev…

e751823

…#24750)

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

TST/CLN: follow-up to pandas-dev#24750 (pandas-dev#24810)

37cf1f3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

STY: use pytest.raises context syntax (series/indexing/*) #24750

STY: use pytest.raises context syntax (series/indexing/*) #24750

simonjayhawkins commented Jan 13, 2019 •

edited

Loading

simonjayhawkins Jan 13, 2019 •

edited

Loading

jreback Jan 13, 2019

simonjayhawkins Jan 13, 2019

jreback Jan 13, 2019

simonjayhawkins Jan 13, 2019

simonjayhawkins Jan 13, 2019

codecov bot commented Jan 13, 2019

codecov bot commented Jan 13, 2019 •

edited

Loading

jreback Jan 13, 2019

jreback Jan 13, 2019

jreback Jan 13, 2019

jreback Jan 13, 2019

simonjayhawkins Jan 13, 2019

jreback Jan 13, 2019

simonjayhawkins Jan 13, 2019

jreback Jan 13, 2019

simonjayhawkins Jan 13, 2019

jreback Jan 13, 2019

jreback Jan 13, 2019

simonjayhawkins Jan 13, 2019

jreback Jan 13, 2019

jreback commented Jan 13, 2019

	@pytest.mark.parametrize("dtype", [np.int64, np.float64])
	def test_where_unsafe_upcast(dtype):
	s = Series(np.arange(10), dtype=dtype)
	values = [2.5, 3.5, 4.5, 5.5, 6.5]

	mask = s < 5
	expected = Series(values + lrange(5, 10), dtype="float64")

	s[mask] = values
	assert_series_equal(s, expected)


	@pytest.mark.xfail
	@pytest.mark.parametrize("dtype", [
	np.int8, np.int16, np.int32, np.float32
	])
	def test_where_unsafe_itemsize_fail(dtype):
	# Can't do these, as we are forced to change the
	# item size of the input to something we cannot.
	s = Series(np.arange(10), dtype=dtype)
	mask = s < 5

	values = [2.5, 3.5, 4.5, 5.5, 6.5]
	with pytest.raises(Exception):
	s[mask] = values

		@@ -165,6 +172,7 @@ def test_where_unsafe_upcast(dtype):
		assert_series_equal(s, expected)


		@pytest.mark.xfail

STY: use pytest.raises context syntax (series/indexing/*) #24750

STY: use pytest.raises context syntax (series/indexing/*) #24750

Conversation

simonjayhawkins commented Jan 13, 2019 • edited Loading

simonjayhawkins Jan 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jan 13, 2019

Codecov Report

codecov bot commented Jan 13, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Jan 13, 2019

simonjayhawkins commented Jan 13, 2019 •

edited

Loading

simonjayhawkins Jan 13, 2019 •

edited

Loading

codecov bot commented Jan 13, 2019 •

edited

Loading