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

Cleaning the constructor of DMDBase subclasses #247

Open
fandreuz opened this issue Jan 22, 2022 · 2 comments
Open

Cleaning the constructor of DMDBase subclasses #247

fandreuz opened this issue Jan 22, 2022 · 2 comments

Comments

@fandreuz
Copy link
Contributor

Every time a new subclass of DMDBase is created there is a lot of recurrent documentation to be copied from another instance to the new one, especially that for __init__(). It might be interested to use the same approach of other libraries (e.g. Matplotlib) and redirect the user to some base class (DMDBase) where the documentation for all the parameters available is stored.

Otherwise we may also store the documentation in a separate class DMDAttributes to reduce the lines dedicated to docstrings in dmdbase.py.

Here are some proposals for the new structure of the constructor:

**kwargs

def __init__(self, **kwargs):
    """
    ...
    Refer to :func:`DMDBase.__init__` for the available parameters.
    """

    self._svd_rank = kwargs['svd_rank']
    ...
  • ✅ Backward compatible
  • ❌ No way to set shared default values
  • ❌ No suggestions in IDE

**kwargs + default dict

def __init__(self, **kwargs):
    """
    ...
    Refer to :func:`DMDBase.__init__` for the available parameters.
    """

    attributes_dict = self.get_default_attributes()
    attributes_dict.update(kwargs)

    self._svd_rank = attributes_dict['svd_rank']
    ...
  • ✅ Backward compatible
  • ✅ Default values
  • ❌ No suggestions in IDE

DMDAttributes

def __init__(self, dmd_attributes):
    """
    ...
    Refer to :func:`DMDBase.__init__` for the available parameters.
    """

    self._svd_rank = dmd_attributes['svd_rank']
    ...
  • ✅ Default values
  • ✅ Suggestions in IDE
  • ❌ Not backward compatible

DMDAttributes + **kwargs + default dict

def __init__(self, dmd_attributes, *, **kwargs):
    """
    ...
    Refer to :func:`DMDBase.__init__` for the available parameters.
    """

    attributes_dict = self.get_default_attributes()
    attributes_dict.update(dmd_attributes.as_dict())
    attributes_dict.update(kwargs)

    self._svd_rank = attributes_dict['svd_rank']
    ...
  • ✅ Backward compatible
  • ✅ Suggestions in IDE if attributes are passed via DMDAttributes
  • ✅ Default values
  • ❌ No suggestions in IDE for kwargs

Note: the star * in the constructor (before **kwargs) is needed because we do not want anything to be given as a non-keyword argument in this kind of setting.

@fandreuz
Copy link
Contributor Author

See also __doc__ attribute (https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html).

@fandreuz fandreuz added this to TODO in Code cleanup Jul 30, 2023
@fandreuz
Copy link
Contributor Author

Dataclases are now very used in the Python environment and there's some tooling we could leverage (e.g. Pydantic).

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

No branches or pull requests

1 participant