Skip to content
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

Use PEP8 method naming conventions and properties #11

Open
Cito opened this issue Oct 17, 2020 · 0 comments
Open

Use PEP8 method naming conventions and properties #11

Cito opened this issue Oct 17, 2020 · 0 comments
Labels
discussion Needs more discussion modernization Update dependencies etc.
Milestone

Comments

@Cito
Copy link
Member

Cito commented Oct 17, 2020

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?

@Cito Cito added this to the 3.1.0 milestone Oct 17, 2020
@Cito Cito added discussion Needs more discussion modernization Update dependencies etc. labels Oct 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Needs more discussion modernization Update dependencies etc.
Projects
None yet
Development

No branches or pull requests

1 participant