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

    https://github.com/apache/spark/pull/16225#discussion_r122164228
  
    --- Diff: python/pyspark/sql/dataframe.py ---
    @@ -1391,9 +1401,10 @@ def all_of_(xs):
                     "to_replace should be a float, int, long, string, list, 
tuple, or dict. "
                     "Got {0}".format(type(to_replace)))
     
    -        if not isinstance(value, valid_types) and not 
isinstance(to_replace, dict):
    +        if not isinstance(value, valid_types) and value is not None \
    --- End diff --
    
    So slightly jet lagged style question: would this be clearer if we just add 
type(None) to L1398? I know PEP8 says we should only use `is` `is not` for 
checking if something is none rather than depending on the implicit conversion 
to boolean -- but since really checking the type here we aren't really in 
danger of that. (This is just a suggestion to make it easier to read - if 
others think its easier to read this way thats fine :)). @davies ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to