zero323 commented on a change in pull request #34433:
URL: https://github.com/apache/spark/pull/34433#discussion_r747342243
##########
File path: python/pyspark/__init__.py
##########
@@ -63,44 +64,56 @@
from pyspark.version import __version__
from pyspark._globals import _NoValue # noqa: F401
+T = TypeVar("T")
+F = TypeVar("F", bound=Callable)
-def since(version):
+
+def since(version: Union[str, float]) -> Callable[[T], T]:
"""
A decorator that annotates a function to append the version of Spark the
function was added.
"""
import re
indent_p = re.compile(r"\n( +)")
- def deco(f):
- indents = indent_p.findall(f.__doc__)
+ def deco(f: T) -> T:
+ indents = indent_p.findall(cast(str, f.__doc__))
indent = " " * (min(len(m) for m in indents) if indents else 0)
- f.__doc__ = f.__doc__.rstrip() + "\n\n%s.. versionadded:: %s" %
(indent, version)
+ f.__doc__ = cast(str, f.__doc__).rstrip() + "\n\n%s.. versionadded::
%s" % (indent, version)
Review comment:
Instead of repeated casts one might `assert` instead:
```python
assert f.__doc__ is not None
indents = indent_p.findall(f.__doc__)
indent = " " * (min(len(m) for m in indents) if indents else 0)
f.__doc__ = f.__doc__.rstrip() + "\n\n%s.. versionadded:: %s" %
(indent, version)
```
##########
File path: python/pyspark/sql/dataframe.py
##########
@@ -2530,7 +2531,9 @@ def replace( # type: ignore[misc]
to_replace: Union[
"LiteralType", List["LiteralType"], Dict["LiteralType",
"OptionalPrimitiveType"]
],
- value: Optional[Union["OptionalPrimitiveType",
List["OptionalPrimitiveType"]]] = _NoValue,
+ value: Optional[
+ Union["OptionalPrimitiveType", List["OptionalPrimitiveType"],
_NoValueType]
+ ] = _NoValue,
Review comment:
I don't think we want to leak this to the end user here, but let's see
what others have to say about it.
FYI @ueshin @xinrong-databricks @itholic
##########
File path: python/pyspark/sql/dataframe.py
##########
@@ -3337,7 +3340,9 @@ def replace(
def replace( # type: ignore[misc]
self,
to_replace: Union[List["LiteralType"], Dict["LiteralType",
"OptionalPrimitiveType"]],
- value: Optional[Union["OptionalPrimitiveType",
List["OptionalPrimitiveType"]]] = _NoValue,
+ value: Optional[
+ Union["OptionalPrimitiveType", List["OptionalPrimitiveType"],
_NoValueType]
+ ] = _NoValue,
Review comment:
Ditto.
##########
File path: python/pyspark/__init__.py
##########
@@ -63,44 +64,55 @@
from pyspark.version import __version__
from pyspark._globals import _NoValue # noqa: F401
+T = TypeVar("T")
+F = TypeVar("F", bound=Callable)
-def since(version):
+def since(version: Union[str, float]) -> Callable[[T], T]:
"""
A decorator that annotates a function to append the version of Spark the
function was added.
"""
import re
indent_p = re.compile(r"\n( +)")
- def deco(f):
- indents = indent_p.findall(f.__doc__)
+ def deco(f: T) -> T:
+ indents = indent_p.findall(cast(str, f.__doc__))
indent = " " * (min(len(m) for m in indents) if indents else 0)
- f.__doc__ = f.__doc__.rstrip() + "\n\n%s.. versionadded:: %s" %
(indent, version)
+ f.__doc__ = cast(str, f.__doc__).rstrip() + "\n\n%s.. versionadded::
%s" % (indent, version)
return f
return deco
-def copy_func(f, name=None, sinceversion=None, doc=None):
+def copy_func(
+ f: F,
+ name: Optional[str] = None,
+ sinceversion: Optional[Union[str, float]] = None,
+ doc: Optional[str] = None,
+) -> F:
"""
Returns a function with same code, globals, defaults, closure, and
name (or provide a new name).
"""
# See
#
http://stackoverflow.com/questions/6527633/how-can-i-make-a-deepcopy-of-a-function-in-python
fn = types.FunctionType(
- f.__code__, f.__globals__, name or f.__name__, f.__defaults__,
f.__closure__
+ f.__code__,
+ f.__globals__, # type: ignore[attr-defined]
Review comment:
Have your tried to follow this suggestion?
> FWIW casting to types.FunctionType should do it, that seems to have all of
these.
https://github.com/python/mypy/issues/1923#issuecomment-234669486
In general, if we have expect `FunctionType` on output, and we clearly
expect it on input, we might consider redefining `TypeVar` here.
##########
File path: python/pyspark/__init__.py
##########
@@ -111,13 +124,13 @@ def keyword_only(func):
"""
@wraps(func)
- def wrapper(self, *args, **kwargs):
+ def wrapper(self: Any, *args: Any, **kwargs: Any) -> Any:
if len(args) > 0:
raise TypeError("Method %s forces keyword arguments." %
func.__name__)
self._input_kwargs = kwargs
return func(self, **kwargs)
- return wrapper
+ return cast(F, wrapper)
Review comment:
Seems like the way to go, but might require some comment.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
