[GitHub] spark pull request #20499: [SPARK-23328][PYTHON] Disallow default value None...

[viirya <gi...@git.apache.org>]

Github user viirya commented on a diff in the pull request:

    https://github.com/apache/spark/pull/20499#discussion_r165828448
  
    --- Diff: python/pyspark/sql/dataframe.py ---
    @@ -1587,6 +1600,51 @@ def replace(self, to_replace, value=None, subset=None):
             |null|  null|null|
             +----+------+----+
             """
    +
    +        # It deals with a problem when 'value' is set to None and 'to_replace' is a dictionary.
    +        is_more_than_two = len(args) + len(kwargs) > 2
    +        if is_more_than_two:
    +            raise TypeError(
    +                "Arguments are expected to be to_replace, value and subset; however, got "
    +                "[%s]" % ", ".join(map(str, [to_replace] + list(args) + list(kwargs.values()))))
    +
    +        is_unexpected_kwargs = \
    +            len(kwargs) == 2 and ("value" not in kwargs or "subset" not in kwargs)
    +        if is_unexpected_kwargs:
    +            raise TypeError(
    +                "Arguments are expected to be to_replace, value and subset; however, got "
    +                "unexpected keyword arguments: [%s]" % ", ".join(
    +                    filter(lambda a: a not in ["value", "subset"], kwargs.keys())))
    +
    +        if "value" in kwargs and "subset" in kwargs:
    +            # replace(..., value=..., subset=...)
    +            # replace(..., subset=..., value=...)
    +            value = kwargs["value"]
    +            subset = kwargs["subset"]
    +        elif "value" in kwargs:
    +            # replace(..., ..., value=...)
    +            value = kwargs["value"]
    +            subset = args[0] if len(args) == 1 else None
    +        elif "subset" in kwargs:
    +            # replace(..., ..., subset=...)
    +            if len(args) == 1:
    +                value = args[0]
    +            elif isinstance(to_replace, dict):
    +                value = None  # When to_replace is a dictionary, value can be omitted.
    +            else:
    +                raise TypeError("value is required when to_replace is not a dictionary.")
    --- End diff --
    
    There are some old check below like:
    ```python
            if not isinstance(value, valid_types) and value is not None \
                    and not isinstance(to_replace, dict):
                raise ValueError("If to_replace is not a dict, value should be "
                                 "a bool, float, int, long, string, list, tuple or None. "
                                 "Got {0}".format(type(value)))
    ```
    
    Should we clean up it too?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org