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

TST: Fix integer ops comparison test #23619

Merged
merged 1 commit into from
Nov 11, 2018

Conversation

TomAugspurger
Copy link
Contributor

The op(Series[integer], other) path was being tested twice.
The op(IntegerArray, other) path was not being tested.

Closes #22096

The `op(Series[integer], other)` path was being tested twice.
The `op(IntegerArray, other)` path was not being tested.

Closes pandas-dev#22096
@pep8speaks
Copy link

Hello @TomAugspurger! Thanks for submitting the PR.

@TomAugspurger TomAugspurger added this to the 0.24.0 milestone Nov 10, 2018
@TomAugspurger TomAugspurger added Testing pandas testing functions or related to the test suite ExtensionArray Extending pandas with custom dtypes or arrays. labels Nov 10, 2018
@codecov
Copy link

codecov bot commented Nov 10, 2018

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #23619   +/-   ##
=======================================
  Coverage   92.25%   92.25%           
=======================================
  Files         161      161           
  Lines       51277    51277           
=======================================
  Hits        47305    47305           
  Misses       3972     3972
Flag Coverage Δ
#multiple 90.63% <ø> (ø) ⬆️
#single 42.32% <ø> (ø) ⬆️

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 8ed92ef...dee6c8c. Read the comment docs.

@jreback jreback merged commit 3230468 into pandas-dev:master Nov 11, 2018
@jreback
Copy link
Contributor

jreback commented Nov 11, 2018

thanks @TomAugspurger

do we have coverage for this anywhere else?

diff --git a/pandas/tests/arrays/test_integer.py b/pandas/tests/arrays/test_integer.py
index 51cd139a6..5e863f61c 100644
--- a/pandas/tests/arrays/test_integer.py
+++ b/pandas/tests/arrays/test_integer.py
@@ -347,6 +347,11 @@ class TestComparisonOps(BaseOpsUtil):
         other = pd.Series([0] * len(data))
         self._compare_other(data, op_name, other)
 
+    def test_compare_ndarray(self, data, all_compare_operators):
+        op_name = all_compare_operators
+        other = np.array(pd.Series([0] * len(data)))
+        self._compare_other(data, op_name, other)
+
 
 class TestCasting(object):
     pass

should we make these ops tests more general? (e.g. i think only integer na exercises them now, but in the future would be nice to automatically test).

thoo added a commit to thoo/pandas that referenced this pull request Nov 11, 2018
* upstream/master:
  BUG: Casting tz-aware DatetimeIndex to object-dtype ndarray/Index (pandas-dev#23524)
  BUG: Delegate more of Excel parsing to CSV (pandas-dev#23544)
  API: DataFrame.__getitem__ returns Series for sparse column (pandas-dev#23561)
  CLN: use float64_t consistently instead of double, double_t (pandas-dev#23583)
  DOC: Fix Order of parameters in docstrings (pandas-dev#23611)
  TST: Unskip some Categorical Tests (pandas-dev#23613)
  TST: Fix integer ops comparison test (pandas-dev#23619)
  DOC: Fixes to docstring to add validation to CI (pandas-dev#23560)
  DOC: Remove incorrect periods at the end of parameter types (pandas-dev#23600)
  MAINT: tm.assert_raises_regex --> pytest.raises (pandas-dev#23592)
  DOC: Updating Series.resample and DataFrame.resample docstrings (pandas-dev#23197)
thoo added a commit to thoo/pandas that referenced this pull request Nov 12, 2018
…fixed

* upstream/master:
  DOC: Enhancing pivot / reshape docs (pandas-dev#21038)
  TST: Fix xfailing DataFrame arithmetic tests by transposing (pandas-dev#23620)
  BUILD: Simplifying contributor dependencies (pandas-dev#23522)
  BUG/REF: TimedeltaIndex.__new__ (pandas-dev#23539)
  BUG: Casting tz-aware DatetimeIndex to object-dtype ndarray/Index (pandas-dev#23524)
  BUG: Delegate more of Excel parsing to CSV (pandas-dev#23544)
  API: DataFrame.__getitem__ returns Series for sparse column (pandas-dev#23561)
  CLN: use float64_t consistently instead of double, double_t (pandas-dev#23583)
  DOC: Fix Order of parameters in docstrings (pandas-dev#23611)
  TST: Unskip some Categorical Tests (pandas-dev#23613)
  TST: Fix integer ops comparison test (pandas-dev#23619)
JustinZhengBC pushed a commit to JustinZhengBC/pandas that referenced this pull request Nov 14, 2018
The `op(Series[integer], other)` path was being tested twice.
The `op(IntegerArray, other)` path was not being tested.

Closes pandas-dev#22096
@TomAugspurger TomAugspurger deleted the integer-na-comparison-test branch November 14, 2018 21:03
@TomAugspurger
Copy link
Contributor Author

We have an open issue for writing base tests comparing arrays to {scalar, list, ndarary, series, index}

tm9k1 pushed a commit to tm9k1/pandas that referenced this pull request Nov 19, 2018
The `op(Series[integer], other)` path was being tested twice.
The `op(IntegerArray, other)` path was not being tested.

Closes pandas-dev#22096
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
The `op(Series[integer], other)` path was being tested twice.
The `op(IntegerArray, other)` path was not being tested.

Closes pandas-dev#22096
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
The `op(Series[integer], other)` path was being tested twice.
The `op(IntegerArray, other)` path was not being tested.

Closes pandas-dev#22096
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ExtensionArray Extending pandas with custom dtypes or arrays. Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants