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

Improve disable_init docs and export the metaclass #55

Merged
merged 4 commits into from
Mar 28, 2025

Conversation

llucax
Copy link
Contributor

@llucax llucax commented Mar 24, 2025

Improve the docs to point out that disabling the __init__ for existing classes via a sub-class is complicated, so it is only recommended to use both the decorator and metaclass on classes inheriting from object directly.

Also make NoInitConstructibleMeta public (remove the _ prefix from the name) and improve its documentation to be more complete and give an example on using with abc.ABCMeta.

llucax added 2 commits March 24, 2025 12:56
Signed-off-by: Leandro Lucarella <[email protected]>
This effectively make it public. We need this in case users want to
combine the metaclass with other metaclasses, like `abc.ABCMeta`.

Signed-off-by: Leandro Lucarella <[email protected]>
@llucax llucax requested a review from a team as a code owner March 24, 2025 11:57
@github-actions github-actions bot added the part:typing Affects the typing module label Mar 24, 2025
@llucax llucax self-assigned this Mar 24, 2025
@llucax llucax added this to the v1.0.0 milestone Mar 24, 2025
@llucax llucax added the cmd:skip-release-notes It is not necessary to update release notes for this PR label Mar 24, 2025
The new recommendation actually doesn't work. The correct way to do it
would be:

```py
super(cls, self).__init__()
```

But this is not even necessary when you are applying the decorator to
a simple class that doesn't inherit from anything (i.e. inherits from
`object` directly).

Applying the decorators to other sub-classes, is much trickier than just
addint the `super()` call, so we will just not recommend or support
doing that and keep the usage documentation simple.

This reverts commit 073b921.

Signed-off-by: Leandro Lucarella <[email protected]>
@llucax llucax force-pushed the improve-disable-init branch from e7161b8 to f1a8db6 Compare March 24, 2025 11:59
@llucax llucax requested a review from Copilot March 24, 2025 11:59
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request improves the documentation for the disable_init decorator and exports the NoInitConstructibleMeta metaclass. Key changes include updating docstrings and examples to clarify the use of the metaclass, removing unnecessary calls to super().init(self) in factory methods, and adjusting keyword arguments handling in error generation functions.

Comments suppressed due to low confidence (1)

src/frequenz/core/typing.py:176

  • [nitpick] The documentation example uses 'meta' to specify the metaclass. Consider using the standard 'metaclass' keyword (i.e. 'class MyClass(metaclass=NoInitConstructibleMeta):') for clarity, if that is supported in your context.
class MyClass(meta=NoInitConstructibleMeta):

Improve the docs to point out that disabling the `__init__` for existing
classes via a sub-class is complicated, so it is only recommended to use
both the decorator and metaclass on classes inheriting from `object`
directly.

Also improve the documentation of `NoInitConstructibleMeta` to be more
complete and give an example on using with `abc.ABCMeta`.

Signed-off-by: Leandro Lucarella <[email protected]>
@llucax llucax force-pushed the improve-disable-init branch from f1a8db6 to cb4c104 Compare March 24, 2025 12:02
@llucax llucax enabled auto-merge March 24, 2025 12:04
@llucax llucax added this pull request to the merge queue Mar 28, 2025
Merged via the queue into frequenz-floss:v1.x.x with commit 4bf37f5 Mar 28, 2025
14 checks passed
@llucax llucax deleted the improve-disable-init branch March 28, 2025 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmd:skip-release-notes It is not necessary to update release notes for this PR part:typing Affects the typing module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants