Skip to content

DOC: Make doc decorator a class and replace Appender by doc #33160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
May 22, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions pandas/tseries/offsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
shift_month,
)
from pandas.errors import AbstractMethodError
from pandas.util._decorators import Appender, Substitution, cache_readonly
from pandas.util._decorators import cache_readonly, doc

from pandas.core.dtypes.inference import is_list_like

Expand Down Expand Up @@ -1188,11 +1188,12 @@ class BusinessMonthBegin(MonthOffset):
_day_opt = "business_start"


@doc(bound="bound")
class _CustomBusinessMonth(_CustomMixin, BusinessMixin, MonthOffset):
"""
DateOffset subclass representing custom business month(s).

Increments between %(bound)s of month dates.
Increments between {bound} of month dates.

Parameters
----------
Expand Down Expand Up @@ -1284,14 +1285,12 @@ def apply(self, other):
return result


@Substitution(bound="end")
@Appender(_CustomBusinessMonth.__doc__)
@doc(_CustomBusinessMonth, bound="end")
class CustomBusinessMonthEnd(_CustomBusinessMonth):
_prefix = "CBM"


@Substitution(bound="beginning")
@Appender(_CustomBusinessMonth.__doc__)
@doc(_CustomBusinessMonth, bound="beginning")
class CustomBusinessMonthBegin(_CustomBusinessMonth):
_prefix = "CBMS"

Expand Down
43 changes: 20 additions & 23 deletions pandas/util/_decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,13 +245,13 @@ def wrapper(*args, **kwargs) -> Callable[..., Any]:
return decorate


def doc(*args: Union[str, Callable], **kwargs: str) -> Callable[[F], F]:
def doc(*args: Union[str, Callable], **kwargs: str) -> Callable[[Callable], Callable]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the names of the parameters, I think a descriptive name is better in this case. I think args and kwargs are great names when they are generic, you can pass whatever, and they are passed to another function mostly.

But in this case, they have very specific usage, and they are very different between them.

I think something like *docs or *docs_or_objs and **params could be ok. May be you think of something better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But in this case, they have very specific usage, and they are very different between them.
I like this point! Agree that we should be specific in this case.

For the names, I think docs and params are good. I also want to add some other candidates here:

  1. use supplements to replace args
  2. use fillers to replace kwargs

I am not a native English speaker, so I am not sure if those words also make sense to you and other developers. Please let me know which one you like and I will be more than happy to make the change.

"""
A decorator take docstring templates, concatenate them and perform string
substitution on it.

This decorator will add a variable "_docstring_components" to the wrapped
function to keep track the original docstring template for potential usage.
callable to keep track the original docstring template for potential usage.
If it should be consider as a template, it will be saved as a string.
Otherwise, it will be saved as callable, and later user __doc__ and dedent
to get docstring.
Expand All @@ -260,40 +260,37 @@ def doc(*args: Union[str, Callable], **kwargs: str) -> Callable[[F], F]:
----------
*args : str or callable
The string / docstring / docstring template to be appended in order
after default docstring under function.
after default docstring under callable.
**kwags : str
The string which would be used to format docstring template.
"""

def decorator(func: F) -> F:
@wraps(func)
def wrapper(*args, **kwargs) -> Callable:
return func(*args, **kwargs)

def decorator(call: Callable) -> Callable:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

call doesn't seem very descriptive to me, may be decorated_obj, func_or_class or something like that?

Is a class of type Callable, even if it doesn't implement __call__? @simonjayhawkins may be you can help, the parameter can be any function or any class, is Callable the appropriate type?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC then this seems OK since we are passing a class type not instance, and a class type is callable in order to instantiate.

from typing import Callable

class Foo:
    pass

def func(call:Callable):
    pass

func(Foo)

reveal_type(Foo)

gives
type-test.py:11: note: Revealed type is 'def () -> type-test.Foo'

and no errors. The class type seems to be treated as a function that returns an instance by mypy.

# collecting docstring and docstring templates
docstring_components: List[Union[str, Callable]] = []
if func.__doc__:
docstring_components.append(dedent(func.__doc__))
if call.__doc__:
docstring_components.append(dedent(call.__doc__))

for arg in args:
if hasattr(arg, "_docstring_components"):
docstring_components.extend(arg._docstring_components) # type: ignore
elif isinstance(arg, str) or arg.__doc__:
docstring_components.append(arg)
for appender in args:
if hasattr(appender, "_docstring_components"):
docstring_components.extend(
appender._docstring_components # type: ignore
)
elif isinstance(appender, str) or appender.__doc__:
docstring_components.append(appender)

# formatting templates and concatenating docstring
wrapper.__doc__ = "".join(
call.__doc__ = "".join(
[
arg.format(**kwargs)
if isinstance(arg, str)
else dedent(arg.__doc__ or "")
for arg in docstring_components
component.format(**kwargs)
if isinstance(component, str)
else dedent(component.__doc__ or "")
for component in docstring_components
]
)

wrapper._docstring_components = docstring_components # type: ignore

return cast(F, wrapper)
call._docstring_components = docstring_components # type: ignore
return call

return decorator

Expand Down