[SPARK-23328][PYTHON] Disallow default value None in na.replace/replace when 'to_replace' is not a dictionary

## What changes were proposed in this pull request?

This PR proposes to disallow default value None when 'to_replace' is not a dictionary.

It seems weird we set the default value of `value` to `None` and we ended up allowing the case as below:

```python
>>> df.show()
```
```
+----+------+-----+
| age|height| name|
+----+------+-----+
|  10|    80|Alice|
...
```

```python
>>> df.na.replace('Alice').show()
```
```
+----+------+----+
| age|height|name|
+----+------+----+
|  10|    80|null|
...
```

**After**

This PR targets to disallow the case above:

```python
>>> df.na.replace('Alice').show()
```
```
...
TypeError: value is required when to_replace is not a dictionary.
```

while we still allow when `to_replace` is a dictionary:

```python
>>> df.na.replace({'Alice': None}).show()
```
```
+----+------+----+
| age|height|name|
+----+------+----+
|  10|    80|null|
...
```

## How was this patch tested?

Manually tested, tests were added in `python/pyspark/sql/tests.py` and doctests were fixed.

Author: hyukjinkwon <gurwls223@gmail.com>

Closes #20499 from HyukjinKwon/SPARK-19454-followup.
This commit is contained in:
hyukjinkwon 2018-02-09 14:21:10 +08:00 committed by Wenchen Fan
parent 8cbcc33876
commit 4b4ee26010
5 changed files with 99 additions and 10 deletions

View file

@ -1929,6 +1929,7 @@ working with timestamps in `pandas_udf`s to get the best performance, see
- The rules to determine the result type of an arithmetic operation have been updated. In particular, if the precision / scale needed are out of the range of available values, the scale is reduced up to 6, in order to prevent the truncation of the integer part of the decimals. All the arithmetic operations are affected by the change, ie. addition (`+`), subtraction (`-`), multiplication (`*`), division (`/`), remainder (`%`) and positive module (`pmod`). - The rules to determine the result type of an arithmetic operation have been updated. In particular, if the precision / scale needed are out of the range of available values, the scale is reduced up to 6, in order to prevent the truncation of the integer part of the decimals. All the arithmetic operations are affected by the change, ie. addition (`+`), subtraction (`-`), multiplication (`*`), division (`/`), remainder (`%`) and positive module (`pmod`).
- Literal values used in SQL operations are converted to DECIMAL with the exact precision and scale needed by them. - Literal values used in SQL operations are converted to DECIMAL with the exact precision and scale needed by them.
- The configuration `spark.sql.decimalOperations.allowPrecisionLoss` has been introduced. It defaults to `true`, which means the new behavior described here; if set to `false`, Spark uses previous rules, ie. it doesn't adjust the needed scale to represent the values and it returns NULL if an exact representation of the value is not possible. - The configuration `spark.sql.decimalOperations.allowPrecisionLoss` has been introduced. It defaults to `true`, which means the new behavior described here; if set to `false`, Spark uses previous rules, ie. it doesn't adjust the needed scale to represent the values and it returns NULL if an exact representation of the value is not possible.
- In PySpark, `df.replace` does not allow to omit `value` when `to_replace` is not a dictionary. Previously, `value` could be omitted in the other cases and had `None` by default, which is counterintuitive and error prone.
## Upgrading From Spark SQL 2.1 to 2.2 ## Upgrading From Spark SQL 2.1 to 2.2

View file

@ -54,6 +54,7 @@ from pyspark.status import *
from pyspark.taskcontext import TaskContext from pyspark.taskcontext import TaskContext
from pyspark.profiler import Profiler, BasicProfiler from pyspark.profiler import Profiler, BasicProfiler
from pyspark.version import __version__ from pyspark.version import __version__
from pyspark._globals import _NoValue
def since(version): def since(version):

View file

@ -0,0 +1,70 @@
#
# Licensed to the Apache Software Foundation (ASF) under one or more
# contributor license agreements. See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You under the Apache License, Version 2.0
# (the "License"); you may not use this file except in compliance with
# the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
"""
Module defining global singleton classes.
This module raises a RuntimeError if an attempt to reload it is made. In that
way the identities of the classes defined here are fixed and will remain so
even if pyspark itself is reloaded. In particular, a function like the following
will still work correctly after pyspark is reloaded:
def foo(arg=pyspark._NoValue):
if arg is pyspark._NoValue:
...
See gh-7844 for a discussion of the reload problem that motivated this module.
Note that this approach is taken after from NumPy.
"""
__ALL__ = ['_NoValue']
# Disallow reloading this module so as to preserve the identities of the
# classes defined here.
if '_is_loaded' in globals():
raise RuntimeError('Reloading pyspark._globals is not allowed')
_is_loaded = True
class _NoValueType(object):
"""Special keyword value.
The instance of this class may be used as the default value assigned to a
deprecated keyword in order to check if it has been given a user defined
value.
This class was copied from NumPy.
"""
__instance = None
def __new__(cls):
# ensure that only one instance exists
if not cls.__instance:
cls.__instance = super(_NoValueType, cls).__new__(cls)
return cls.__instance
# needed for python 2 to preserve identity through a pickle
def __reduce__(self):
return (self.__class__, ())
def __repr__(self):
return "<no value>"
_NoValue = _NoValueType()

View file

@ -27,7 +27,7 @@ else:
import warnings import warnings
from pyspark import copy_func, since from pyspark import copy_func, since, _NoValue
from pyspark.rdd import RDD, _load_from_socket, ignore_unicode_prefix from pyspark.rdd import RDD, _load_from_socket, ignore_unicode_prefix
from pyspark.serializers import ArrowSerializer, BatchedSerializer, PickleSerializer, \ from pyspark.serializers import ArrowSerializer, BatchedSerializer, PickleSerializer, \
UTF8Deserializer UTF8Deserializer
@ -1532,7 +1532,7 @@ class DataFrame(object):
return DataFrame(self._jdf.na().fill(value, self._jseq(subset)), self.sql_ctx) return DataFrame(self._jdf.na().fill(value, self._jseq(subset)), self.sql_ctx)
@since(1.4) @since(1.4)
def replace(self, to_replace, value=None, subset=None): def replace(self, to_replace, value=_NoValue, subset=None):
"""Returns a new :class:`DataFrame` replacing a value with another value. """Returns a new :class:`DataFrame` replacing a value with another value.
:func:`DataFrame.replace` and :func:`DataFrameNaFunctions.replace` are :func:`DataFrame.replace` and :func:`DataFrameNaFunctions.replace` are
aliases of each other. aliases of each other.
@ -1545,8 +1545,8 @@ class DataFrame(object):
:param to_replace: bool, int, long, float, string, list or dict. :param to_replace: bool, int, long, float, string, list or dict.
Value to be replaced. Value to be replaced.
If the value is a dict, then `value` is ignored and `to_replace` must be a If the value is a dict, then `value` is ignored or can be omitted, and `to_replace`
mapping between a value and a replacement. must be a mapping between a value and a replacement.
:param value: bool, int, long, float, string, list or None. :param value: bool, int, long, float, string, list or None.
The replacement value must be a bool, int, long, float, string or None. If `value` is a The replacement value must be a bool, int, long, float, string or None. If `value` is a
list, `value` should be of the same length and type as `to_replace`. list, `value` should be of the same length and type as `to_replace`.
@ -1577,6 +1577,16 @@ class DataFrame(object):
|null| null|null| |null| null|null|
+----+------+----+ +----+------+----+
>>> df4.na.replace({'Alice': None}).show()
+----+------+----+
| age|height|name|
+----+------+----+
| 10| 80|null|
| 5| null| Bob|
|null| null| Tom|
|null| null|null|
+----+------+----+
>>> df4.na.replace(['Alice', 'Bob'], ['A', 'B'], 'name').show() >>> df4.na.replace(['Alice', 'Bob'], ['A', 'B'], 'name').show()
+----+------+----+ +----+------+----+
| age|height|name| | age|height|name|
@ -1587,6 +1597,12 @@ class DataFrame(object):
|null| null|null| |null| null|null|
+----+------+----+ +----+------+----+
""" """
if value is _NoValue:
if isinstance(to_replace, dict):
value = None
else:
raise TypeError("value argument is required when to_replace is not a dictionary.")
# Helper functions # Helper functions
def all_of(types): def all_of(types):
"""Given a type or tuple of types and a sequence of xs """Given a type or tuple of types and a sequence of xs
@ -2047,7 +2063,7 @@ class DataFrameNaFunctions(object):
fill.__doc__ = DataFrame.fillna.__doc__ fill.__doc__ = DataFrame.fillna.__doc__
def replace(self, to_replace, value, subset=None): def replace(self, to_replace, value=_NoValue, subset=None):
return self.df.replace(to_replace, value, subset) return self.df.replace(to_replace, value, subset)
replace.__doc__ = DataFrame.replace.__doc__ replace.__doc__ = DataFrame.replace.__doc__

View file

@ -2243,11 +2243,6 @@ class SQLTests(ReusedSQLTestCase):
.replace(False, True).first()) .replace(False, True).first())
self.assertTupleEqual(row, (True, True)) self.assertTupleEqual(row, (True, True))
# replace list while value is not given (default to None)
row = self.spark.createDataFrame(
[(u'Alice', 10, 80.0)], schema).replace(["Alice", "Bob"]).first()
self.assertTupleEqual(row, (None, 10, 80.0))
# replace string with None and then drop None rows # replace string with None and then drop None rows
row = self.spark.createDataFrame( row = self.spark.createDataFrame(
[(u'Alice', 10, 80.0)], schema).replace(u'Alice', None).dropna() [(u'Alice', 10, 80.0)], schema).replace(u'Alice', None).dropna()
@ -2283,6 +2278,12 @@ class SQLTests(ReusedSQLTestCase):
self.spark.createDataFrame( self.spark.createDataFrame(
[(u'Alice', 10, 80.1)], schema).replace({u"Alice": u"Bob", 10: 20}).first() [(u'Alice', 10, 80.1)], schema).replace({u"Alice": u"Bob", 10: 20}).first()
with self.assertRaisesRegexp(
TypeError,
'value argument is required when to_replace is not a dictionary.'):
self.spark.createDataFrame(
[(u'Alice', 10, 80.0)], schema).replace(["Alice", "Bob"]).first()
def test_capture_analysis_exception(self): def test_capture_analysis_exception(self):
self.assertRaises(AnalysisException, lambda: self.spark.sql("select abc")) self.assertRaises(AnalysisException, lambda: self.spark.sql("select abc"))
self.assertRaises(AnalysisException, lambda: self.df.selectExpr("a + b")) self.assertRaises(AnalysisException, lambda: self.df.selectExpr("a + b"))