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

Fix the whole class attribute shenanigans #274

Open
Cimbali opened this issue Mar 9, 2023 · 2 comments
Open

Fix the whole class attribute shenanigans #274

Cimbali opened this issue Mar 9, 2023 · 2 comments

Comments

@Cimbali
Copy link
Owner

Cimbali commented Mar 9, 2023

Thanks to @f0k for bringing this up in #273:

[T]he original code [style is] to create a class attribute media_type that is later shadowed by an instance attribute. (Which may or may not have been intentional, see https://www.bruceeckel.com/2022/05/11/misunderstanding-python-class-attributes/. […])

It is indeed intentional -- I’ve inherited the code sort of like that I believe. It allowed for a better oversight and documentation of the code, in a more declarative way than having to do it all in a very long __init__().

I know it means an (unitialized) Class.attribute can be accessed instead of object.attribute, or object.attribute really returns Class.attribute whenever there is no initialization in __init__().

This could probably bite us in the arse at some point as most classes are singletons so it’s not really something I look out for. One of the classes that isn’t a singleton could show unexpected behaviour by modifying a non-inited attribute (without setting it, e.g. add an element to a dict), which would modify every object’s dict (as they all really access the class attribute and not their own property).

We probably should look at fixing this. Maybe a wrapper or metaclass solution that at the end of __init__ just makes a copy of all missing class attributes into the object’s properties. (Plus a way to mark some as static?)

Alternatives that keep things documented would be

  1. to make every property a property class, which allows docstrings, or
  2. move everything inside __init__() and put docstrings on the self.x = ... statements.

I’m not very fond of the former which would make everything harder to read. Both require very noisy changes. I’m surprised that option 2 even works but I checked -- Sphinx does access everything inside __init__().

But the discussion is open for better solutions as I understand that basically making python behave like non-python languages may be unintuitive. (To be fair, unintuitive to people who know python in and out, and more intuitive to people coming to python from different language backgrounds.)


In any case once this is fixed, a proper code style documentation should also be written up and put in a visible place -- we currently only have a 2-3 line paragraph hidden in an obscure docs page:

The code is documented in the source using the Google style for docstrings. Sphinx has gathered a set of examples which serves as a better crash course than the full style reference.

Retructured text (rst) can be used inside the comments and docstrings.

@Cimbali
Copy link
Owner Author

Cimbali commented Mar 9, 2023

The following example (slightly tweaked) from Sphinx’s doc:

class Foo:
    """Docstring for class Foo."""

    #: Doc comment for class attribute Foo.bar.
    #: It can have multiple lines.
    bar = 1

    flox = 1.5   #: Doc comment for Foo.flox. One line only.

    baz = 2
    """Docstring for class attribute Foo.baz."""

    def further_init(self):
        """ A function doing more complex init """
        #: Doc comment for instance attribute qux -- is ignored by sphinx
        self.qux = 3

    def __init__(self):
        self.spam = 4
        """Docstring for instance attribute spam."""

        self.further_init()

Gives the following doc output (as text):

test module
***********

class test.Foo

   Bases: "object"

   Docstring for class Foo.

   bar = 1

      Doc comment for class attribute Foo.bar. It can have multiple lines.

   baz = 2

      Docstring for class attribute Foo.baz.

   flox = 1.5

      Doc comment for Foo.flox. One line only.

   further_init()

      A function doing more complex init

   spam

      Docstring for instance attribute spam.

@Cimbali
Copy link
Owner Author

Cimbali commented Mar 9, 2023

A metaclass solution would look like this:

import copy
import functools

class PropertyInitializer(type):
    """ A meta class that modifies a class’s __init__ to initialize all class attributes on an object as properties

    With the exception of:
        - names set into class._classmembers
        - methods (lambda attributes on the other hand are copied)
        - properties already initialized in that class’s __init__()
    """
    def attribute_copy_init(cls, init, classmembers):
        """ Wrap class `~cls`’s `__init__` function `~init` and copy attributes

        Args:
            cls (`class`): The class on which the initialization has to be done
            init (`function`): The wrapped __init__ function
            classmembers (`tuple` of `str`): The names of class attributes that should be accessed statically
        """
        # Check we are not requesting unreasonable things
        undefined = set(classmembers) - set(vars(cls).keys())
        if undefined:
            raise NameError('classmembers are not defined in class {}: {}'.format(cls.__name__, ', '.join(undefined)))

        @functools.wraps(init)
        def wrapper(self, *args, **kwargs):
            """ Inner __init__ wrapper """
            # First do normal initialization
            init(self, *args, **kwargs)

            # Now copy over class attributes as object properties
            for key, value in vars(cls).items():
                if key[:2] + key[-2:] == '____':
                    # don’t mess with double underscores
                    continue
                if key in vars(self) or key in classmembers:
                    # initialized in __init__() or static
                    continue
                if callable(value) and getattr(value, '__name__', '') != '<lambda>':
                    # A method (and not a lambda attribute), don’t want to overwrite those
                    continue
                # Importantly, use copy(), so we don’t keep the same reference
                setattr(self, key, copy.copy(getattr(cls, key)))
        return wrapper

    def __new__(cls, name, bases, attrs, classmembers=()):
        created_class = super().__new__(cls, name, bases, attrs)
        created_class.__init__ = cls.attribute_copy_init(created_class, created_class.__init__, classmembers)
        return created_class

Then change class declarations to have class X(metaclass=PropertyInitializer):

A decorator solution is possible too, but makes it look like every class inherits just from itself
import functools
import copy

def init_properties(classmembers=()):
    """ A class decorator to initialize all properties on an object that are declared as class attributes

    With the exception of:
        - names passed into classmembers
        - methods (lambda attributes on the other hand are copied)
        - properties already initialized in that class’s __init__()

    Args:
        classmembers (`tuple` of `str`): Names of attributes in the wrapped class that should be accessed statically
    """
    def wrap(cls):
        """ Inner wrapper.

        Args:
            cls (`class`): The wrapped class
        """
        # Check we are not requesting unreasonable things
        undefined = set(classmembers) - set(vars(cls).keys())
        if undefined:
            raise NameError('classmembers are not defined in class {}: {}'.format(cls.__name__, ', '.join(undefined)))

        @functools.wraps(cls, updated=())
        class InitializingClass(cls):
            """ A class wrapping and inheriting from `~cls`, that additionally initializes properties """
            def __init__(self, *args, **kwargs):
                # First do normal initialization
                super(InitializingClass, self).__init__(*args, **kwargs)

                # Now copy over class attributes as object properties
                for key, value in vars(cls).items():
                    if key[:2] + key[-2:] == '____':
                        # don’t mess with double underscores
                        continue
                    elif key in vars(self) or key in classmembers:
                        # initialized in __init__() or static
                        continue
                    elif callable(value) and getattr(value, '__name__', '') != '<lambda>':
                        # A method (and not a lambda attribute), don’t want to overwrite those
                        continue
                    else:
                        # Importantly, use copy(), so we don’t keep the same reference
                        setattr(self, key, copy.copy(getattr(cls, key)))
        return InitializingClass
    return wrap

Then prepend @init_properties() before every class declaration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant