You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/07/21 22:11:45 UTC
[GitHub] [spark] xinrong-databricks commented on a change in pull request #33402: [SPARK-36189][PYTHON] Improve bool, string, numeric DataTypeOps tests by avoiding joins
xinrong-databricks commented on a change in pull request #33402:
URL: https://github.com/apache/spark/pull/33402#discussion_r674379508
##########
File path: python/pyspark/pandas/tests/data_type_ops/test_boolean_ops.py
##########
@@ -24,289 +24,273 @@
from pandas.api.types import CategoricalDtype
from pyspark import pandas as ps
-from pyspark.pandas.config import option_context
from pyspark.pandas.tests.data_type_ops.testing_utils import TestCasesUtils
from pyspark.pandas.typedef.typehints import (
extension_float_dtypes_available,
extension_object_dtypes_available,
)
-from pyspark.sql.types import BooleanType
from pyspark.testing.pandasutils import PandasOnSparkTestCase
class BooleanOpsTest(PandasOnSparkTestCase, TestCasesUtils):
@property
- def pser(self):
- return pd.Series([True, True, False])
+ def bool_pdf(self):
+ return pd.DataFrame({"this": [True, False, True], "that": [False, True, True]})
@property
- def psser(self):
- return ps.from_pandas(self.pser)
-
- @property
- def float_pser(self):
- return pd.Series([1, 2, 3], dtype=float)
-
- @property
- def float_psser(self):
- return ps.from_pandas(self.float_pser)
-
- @property
- def other_pser(self):
- return pd.Series([False, False, True])
-
- @property
- def other_psser(self):
- return ps.from_pandas(self.other_pser)
+ def bool_psdf(self):
+ return ps.from_pandas(self.bool_pdf)
def test_add(self):
- pser = self.pser
- psser = self.psser
- self.assert_eq(pser + 1, psser + 1)
- self.assert_eq(pser + 0.1, psser + 0.1)
- self.assert_eq(pser + pser.astype(int), psser + psser.astype(int))
- self.assert_eq(pser + pser, psser + psser)
- self.assert_eq(pser + True, psser + True)
- self.assert_eq(pser + False, psser + False)
-
- with option_context("compute.ops_on_diff_frames", True):
- for pser, psser in self.numeric_pser_psser_pairs:
- self.assert_eq(self.pser + pser, (self.psser + psser).sort_index())
-
- for pser, psser in self.non_numeric_pser_psser_pairs:
- if isinstance(psser.spark.data_type, BooleanType):
- self.assert_eq(self.pser + pser, (self.psser + psser).sort_index())
- else:
- self.assertRaises(TypeError, lambda: self.psser + psser)
+ pdf, psdf = self.pdf, self.psdf
+
+ b_pser, b_psser = pdf["bool"], psdf["bool"]
+ self.assert_eq(b_pser + 1, b_psser + 1)
+ self.assert_eq(b_pser + 0.1, b_psser + 0.1)
+ self.assert_eq(b_pser + b_pser.astype(int), b_psser + b_psser.astype(int))
+ self.assert_eq(b_pser + b_pser, b_psser + b_psser)
+ self.assert_eq(b_pser + True, b_psser + True)
+ self.assert_eq(b_pser + False, b_psser + False)
+
+ for col in self.numeric_df_cols:
+ pser, psser = pdf[col], psdf[col]
+ self.assert_eq(b_pser + pser, b_psser + psser)
+ for col in self.non_numeric_df_cols:
+ pser, psser = pdf[col], psdf[col]
+ if col == "bool":
+ self.assert_eq(b_pser + pser, b_psser + psser)
+ else:
+ self.assertRaises(TypeError, lambda: b_psser + psser)
def test_sub(self):
- pser = self.pser
- psser = self.psser
- self.assert_eq(pser - 1, psser - 1)
- self.assert_eq(pser - 0.1, psser - 0.1)
- self.assert_eq(pser - pser.astype(int), psser - psser.astype(int))
- self.assertRaises(TypeError, lambda: psser - psser)
- self.assertRaises(TypeError, lambda: psser - True)
+ pdf, psdf = self.pdf, self.psdf
+
+ b_pser, b_psser = pdf["bool"], psdf["bool"]
+ self.assert_eq(b_pser - 1, b_psser - 1)
+ self.assert_eq(b_pser - 0.1, b_psser - 0.1)
+ self.assert_eq(b_pser - b_pser.astype(int), b_psser - b_psser.astype(int))
+ self.assertRaises(TypeError, lambda: b_psser - b_psser)
+ self.assertRaises(TypeError, lambda: b_psser - True)
- with option_context("compute.ops_on_diff_frames", True):
- for pser, psser in self.numeric_pser_psser_pairs:
- self.assert_eq(self.pser - pser, (self.psser - psser).sort_index())
+ for col in self.numeric_df_cols:
+ self.assert_eq(b_pser - pdf[col], b_psser - psdf[col])
- for psser in self.non_numeric_pssers.values():
- self.assertRaises(TypeError, lambda: self.psser - psser)
+ for col in self.non_numeric_df_cols:
+ self.assertRaises(TypeError, lambda: b_psser - psdf[col])
def test_mul(self):
- pser = self.pser
- psser = self.psser
- self.assert_eq(pser * 1, psser * 1)
- self.assert_eq(pser * 0.1, psser * 0.1)
- self.assert_eq(pser * pser.astype(int), psser * psser.astype(int))
- self.assert_eq(pser * pser, psser * psser)
- self.assert_eq(pser * True, psser * True)
- self.assert_eq(pser * False, psser * False)
-
- with option_context("compute.ops_on_diff_frames", True):
Review comment:
https://github.com/apache/spark/blob/master/python/pyspark/pandas/tests/test_ops_on_diff_frames.py are with `compute.ops_on_diff_frames` option on. Would you think that's enough? If not, I would like to follow up on this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org