Yikun commented on PR #36547: URL: https://github.com/apache/spark/pull/36547#issuecomment-1128324461
> But if you look at the code that pandas are using, they [decorate the function](https://github.com/pandas-dev/pandas/blob/v1.4.2/pandas/core/groupby/groupby.py#L1810) with @final Actually, the `final` is not for the decoration of built-in function in Pandas, it `to indicate to the reader that a the method is not overridden` according https://github.com/pandas-dev/pandas/commit/7073ee1612019f4e0e9d6a1b205257e8eab02382 merge history. As @ueshin mentioned, there also are no issue when using build-in same name func under the namespace, this is also the recommand way to [pep-20](https://peps.python.org/pep-0020/#the-zen-of-python): `Namespaces are one honking great idea -- let's do more of those!`. > Spark python codebase has no more than 56 shading python built-in functions now, which can be a problem. The only potential issue I can imagine, is for Spark dev (not users), we have to use `builtins.set` when we want to call the built-in under the same namespace, such as: ```python import builtins from typing import final class A(): def __init__(self, value): self.set(value) # class method with same name def set(self, value): self.value=value # TypeError: set() missing 1 required positional argument: 'value' # test_val = set([1,2,3]) # This is ok, becasue we are using set under builtins namespace test_val = builtins.set([1,2,3]) ``` As above demo, even we add `final`, we still can't avoid this case, we should using `builtins.set` to resolve the namespace problems. BTW, If we need to add `final` (if we think `to indicate to the reader that a the method is not overridden` is very important), we could add this later in separate patch. I'm OK but doesn't have very strong feel with 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-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
