Python: Deprecate resource name helper functions

[busunkim96]

These functions will be going away in the microgenerator.

[yihanzhen]

Is this a proper way to deprecated a method in Python?

@classmethod
def book_path(cls, book_shelf, book):
    """Return a fully-qualified book string."""
    import warnings
    warnings.warn("resource name helper functions are deprecated", DeprecationWarning,
              stacklevel=1)
    return google.api_core.path_template.expand(
        'bookShelves/{book_shelf}/books/{book}',
        book_shelf=book_shelf,
        book=book,
    )

[software-dov]

from deprecated import deprecated


@deprecated
def print_value(value):
    """
    Print the value

    :param value: The value to print
    """
    pprint.pprint(value)

https://python-deprecated.readthedocs.io/en/latest/installation.html

If we don't want the extra dependency it's easy to write a custom decorator that does the same thing.

[busunkim96]

@hzyi-google that looks to be how stuff has been deprecated in the past:

https://github.com/googleapis/google-cloud-python/blob/e88d543c194fb610c56615f354e270f4f3648412/vision/google/cloud/vision_v1p1beta1/gapic/image_annotator_client.py#L118-L134

[busunkim96]

@software-dov I would lean towards not adding the extra dependency.

The deprecation should also be noted in the docstring (so it is visible in the published documentation).

@classmethod
def book_path(cls, book_shelf, book):
    """DEPRECATED. Return a fully-qualified book string."""
    warnings.warn("resource name helper functions are deprecated", DeprecationWarning,
              stacklevel=1)
    return google.api_core.path_template.expand(
        'bookShelves/{book_shelf}/books/{book}',
        book_shelf=book_shelf,
        book=book,
    )

[software-dov]

Something like

from functools import wraps
import warnings
def deprecated(func):
    @wraps(func)
    def wrapper(*args, **kwargs):
        warnings.warn("Deprecated: " + func.__name__, DeprecationWarning, stacklevel=2)
        return func(*args, **kwargs)

    wrapper.__doc__ = ("DEPRECATED. " + (wrapper.__doc__ or "")).strip()
    return wrapper

should work. Just a pet peeve, but a decorator feels like a better fit than generating all the warnings in situ.

[busunkim96]

👍 I have no strong feelings on the implementation, as long as there is (1) a warning and (2) something in the docstring.

Would adding the decorator to google-api-core work?

CC @crwilcox @tseaver @tswast who may have more opinions about this.

[software-dov]

It sounds fine to have the decorator in google-api-core, but other people should weigh in too.

[tseaver]

Seems like gratuitous breakage to drop the name / path helper functions in the microgenerator: what is the rationale?