WebwareForPython / w4py3

Webware for Python 3
MIT License
20 stars 6 forks source link

Use PEP8 method naming conventions and properties #11

Open Cito opened 4 years ago

Cito commented 4 years ago

Currently, the API of Webware 3.0 still uses the traditional naming conventions and the conventions for getters and setters that originated from times when there was no PEP8 and Python had no support for properties.

If we want to modernize the API of Webware for Python, and make it more Pythonic again, it makes sense to tackle these two issues in combination.

In a first step, we would try to maintain full backward compatibility and print deprecation warnings if the old names are used. In a second step (which should happen as a new major version) we could then desupport the old API.

For modules, we could achieve this by adding stubs with the old names that would import everything from the new module and print a deprecation warning telling the user to import the new module (e.g. http_content instead of HTTPContent).

For camel-cased method names like servletPath() we can simply add an equivalent servlet_path property. When you call servletPath() you would get a deprecation warning that you should use servlet_path instead.

For simple non camel-cased method names like request() this is a bit tricky because old and new names are the same, and we only want to make it a property instead of a getter. As a solution, we could make request a property instead of a method, but also make the Request objects callable, and when called return the object itself and print a deprecation warning. Same with response, session etc. We could also generalize this to any objects returned by getters, including strings. The trick should work for any getters that return non-callable objects. We will need to make sure that there aren't any getters with simple names in the public API that return callable objects, and provide different solutions for these if they exist.

We could then simply write servlet.request.url instead of servlet.request().url(), but the latter would still work and do the same, except for printing deprecation warnings.

This could be implemented as follows:

import warnings

def deprecated_getter(new_name):
    warnings.warn(
        "Calling deprecated getter method"
        " instead of using property '%s'." % (new_name,),
        category=DeprecationWarning, stacklevel=3)

def deprecated_setter(new_name):
    warnings.warn(
        "Calling deprecated setter method"
        " instead of using property '%s'." % (new_name,),
        category=DeprecationWarning, stacklevel=3)

class GettableMixin:

    def __call__(self):
        warnings.warn(
            "Calling deprecated getter method"
            " instead of using property with the same name.",
            category=DeprecationWarning, stacklevel=2)
        return self

class GettableStr(GettableMixin, str):
    """A gettable str type."""

class Request:

    def __init__(self):
        self._url = '/test/url'
        self._servlet_path = '/www/servlets'

    # example of attribute without camel case

    @property
    def url(self):
        return GettableStr(self._url)

    @url.setter
    def url(self, url):
        self._url = url

    def setUrl(self, url):
        deprecated_setter(new_name='url')
        self.url = url

    # example of attribute with camel case

    @property
    def servlet_path(self):
        return self._servlet_path

    @servlet_path.setter
    def servlet_path(self, servlet_path):
        self._servlet_path = servlet_path

    def servletPath(self):
        deprecated_getter(new_name='servlet_path')
        return self.servlet_path

    def setServletPath (self, servlet_path):
        deprecated_setter(new_name='servlet_path')
        self.servlet_path = servlet_path

warnings.simplefilter('always', DeprecationWarning)

request = Request()

print(request.url)
print(request.url())
request.url = 'test2/url2'
print(request.url)
request.setUrl('test3/url3')
print(request.url)

print(request.servlet_path)
print(request.servletPath())
request.servlet_path = '/www/servlets2'
print(request.servlet_path)
request.setServletPath('/www/servlets3')
print(request.servlet_path)

Creating methods with the old names and deprecation warning could be implemented as follows:

import functools
import warnings

def deprecated_method(old_name, new_method):

    @functools.wraps(new_method)
    def old_method(*args, **kwargs):
        warnings.warn("Method {} has been renamed to {}.".format(
            old_name, new_method.__name__),
            category=DeprecationWarning, stacklevel=2)
        return new_method(*args, **kwargs)

    return old_method

def add_deprecated_aliases(cls):
    aliases = cls._deprecated_aliases
    for old_name, new_method in aliases.items():
        old_method = deprecated_method(old_name, new_method)
        setattr(cls, old_name, old_method)
    return cls

@add_deprecated_aliases
class Something:

    def __init__(self):
        self._url = '/test/url'
        self._servlet_path = '/www/servlets'

    # example of arbitrary deprecated method

    def do_something(self):
        print("Doing something...")

    _deprecated_aliases = dict(
        doSomething=do_something)

warnings.simplefilter('always', DeprecationWarning)

thing = Something()

print("Doing it the new way:")
thing.do_something()
print("Doing it the old way:")
thing.doSomething()

Or, the new properties could be generated automatically with such a function:

def makeNewStyle(cls):
    for attr in dir(cls):
        if attr.startswith('_') or not attr[:1].islower():
            continue
        if attr.startswith('set') and attr[3:4].isupper():
            continue
        if attr.startswith('del') and attr[3:4].isupper():
            continue
        getter = getattr(cls, attr)
        if not callable(getter):
            continue
        if attr.startswith('get') and attr[3:4].isupper():
            capAttr = attr[3:]
            getAttr = None
        else:
            capAttr = attr.capitalize()
            getAttr = 'get' + capAttr
            if hasattr(cls, getAttr):
                continue
        setAttr = 'set' + capAttr
        setter = getattr(cls, setAttr, None)
        if setter and not callable(setter):
            continue
        delAttr = 'del' + capAttr
        deleter = getattr(cls, delAttr, None)
        if deleter and not callable(deleter):
            continue
        if getAttr:
            setattr(cls, getAttr, getter)
        attr = capAttr[:1].lower() + capAttr[1:]
        setattr(cls, attr, property(getter, setter, deleter, getter.__doc__))

Now we only need to combine the generation of the properties and the addition of the GettableMixin from above. This may require some hints which type of objects a method returns. We could e.g. use type hints to do that.

So there is a way forward to modernize the API and make it more Pythonic again in a two-step process using the ideas outlined above. Is it worth the effort, though?