tvalentyn commented on code in PR #28396:
URL: https://github.com/apache/beam/pull/28396#discussion_r1322263429
##########
sdks/python/apache_beam/dataframe/frame_base.py:
##########
@@ -484,18 +484,43 @@ def args_to_kwargs(base_type):
determine the name to use for arguments that are converted to keyword
arguments.
- For internal use only. No backwards compatibility guarantees."""
+ removed_method used in cases where a method has been removed in a later
+ version of Pandas. removed_args used in cases where a method has had
+ arguments removed in a later version of Pandas.
+
+ For internal use only. No backwards compatibility guarantees.
+ """
def wrap(func):
- arg_names = getfullargspec(unwrap(getattr(base_type, func.__name__))).args
+ if removed_method:
+ # Do no processing, let Beam function itself raise the error if called.
+ return func
+
+ removed_arg_names = removed_args if removed_args is not None else []
+
+ base_arg_spec = getfullargspec(unwrap(getattr(base_type, func.__name__)))
+ base_arg_names = base_arg_spec.args
+ # Some arguments are keyword only and we still want to check against those.
+ all_possible_base_arg_names = base_arg_names + base_arg_spec.kwonlyargs
+ beam_arg_names = getfullargspec(func).args
+
+ if not_found := (set(beam_arg_names) - set(all_possible_base_arg_names) -
+ set(removed_arg_names)):
+ raise TypeError(
+ f"Beam definition of {func.__name__} has arguments that are not
found"
+ f" in the base version of the function: {not_found}")
@functools.wraps(func)
def wrapper(*args, **kwargs):
- for name, value in zip(arg_names, args):
+ for name, value in zip(base_arg_names, args):
if name in kwargs:
raise TypeError(
"%s() got multiple values for argument '%s'" %
(func.__name__, name))
kwargs[name] = value
+ # Still have to populate these for the Beam function signature.
+ if removed_args:
+ for name in removed_args:
+ kwargs[name] = None
Review Comment:
should we pop from kwargs instead of assigning None?
##########
sdks/python/apache_beam/dataframe/frame_base.py:
##########
@@ -484,18 +484,43 @@ def args_to_kwargs(base_type):
determine the name to use for arguments that are converted to keyword
arguments.
- For internal use only. No backwards compatibility guarantees."""
+ removed_method used in cases where a method has been removed in a later
+ version of Pandas. removed_args used in cases where a method has had
+ arguments removed in a later version of Pandas.
+
+ For internal use only. No backwards compatibility guarantees.
+ """
def wrap(func):
- arg_names = getfullargspec(unwrap(getattr(base_type, func.__name__))).args
+ if removed_method:
+ # Do no processing, let Beam function itself raise the error if called.
Review Comment:
will the error self-explanatory to the user?
--
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]
