janluke / cloup

Library to build command line interfaces based on Click. It extends click with: option groups, constraints (e.g. mutually exclusive params), command aliases, help themes, "did you mean ...?" suggestions and more.
https://cloup.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
104 stars 10 forks source link

`HelpTheme` as `dataclass` rather than `NamedTuple` #163

Closed janluke closed 1 year ago

janluke commented 1 year ago

Fixes #159.

@kdeldycke Does this solve your issue?

kdeldycke commented 1 year ago

I tried to directly inherits from your new HelpTheme dataclass, but of course this is not allowed:

TypeError: cannot inherit non-frozen dataclass from a frozen one 😅

janluke commented 1 year ago

Do you need it to be a non-frozen dataclass?

kdeldycke commented 1 year ago

Do you need it to be a non-frozen dataclass?

Unfreezing it would allow me to streamline my code with:

@dataclass
class HelpExtraTheme(cloup.HelpTheme):

    critical: IStyle = identity
    error: IStyle = identity
    warning: IStyle = identity
    info: IStyle = identity
    debug: IStyle = identity
    """Log levels from Python's logging module."""

    ...

Else I have to build up a new class programmatically out of cloup.HelpTheme fields.

Freezing that dataclass is tempting and I understand the appeal. But I guess that's not critical. I'm probably the only one on the planet trying to play with the internals of cloup.HelpTheme. So no need to protect me from myself! 😇 I'll take full responsibility and not blame you if I mess up! 😁

janluke commented 1 year ago

Sorry, it's still not clear to me why you can't

@dataclass(frozen=True)
class HelpExtraTheme(cloup.HelpTheme):
    ...
kdeldycke commented 1 year ago

Sorry, it's still not clear to me why you can't

Because I'm stupid! 😓

I apologize for the back and forth, I'm just discovering this implementation detail. You are right: it is indeed possible to subclass a frozen dataclass. All you need to make it frozen too. And mine wasn't.

Here is the proof:

Python 3.11.4 (main, Jun 20 2023, 17:23:00) [Clang 14.0.3 (clang-1403.0.22.14.1)] on darwin
Type "help", "copyright", "credits" or "license" for more information.

>>> from dataclasses import dataclass

>>> @dataclass(frozen=True)
... class HelpTheme:
...     ...
... 

>>> HelpTheme
<class '__main__.HelpTheme'>

>>> @dataclass
... class HelpExtraTheme(HelpTheme):
...     ...
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File ".../python3.11/dataclasses.py", line 1230, in dataclass
    return wrap(cls)
           ^^^^^^^^^
  File ".../python3.11/dataclasses.py", line 1220, in wrap
    return _process_class(cls, init, repr, eq, order, unsafe_hash,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../python3.11/dataclasses.py", line 988, in _process_class
    raise TypeError('cannot inherit non-frozen dataclass from a '
TypeError: cannot inherit non-frozen dataclass from a frozen one

>>> @dataclass(frozen=True)
... class HelpExtraTheme(HelpTheme):
...     ...
... 

>>> HelpExtraTheme
<class '__main__. HelpExtraTheme'>

Just tried this branch against my code and it works! 🎉

See: https://github.com/kdeldycke/click-extra/pull/704/files

So yeah, you can merge this PR. LGTM!

kdeldycke commented 1 year ago

Also, I had to redefine the with_ helper. My version is a little convoluted but generic. Feel free to adopt it if it makes sense for Cloup: https://github.com/kdeldycke/click-extra/blob/f93cbc8ddfa8b8cc135f54474929bd74cf57ec7f/click_extra/colorize.py#L87-L109

kdeldycke commented 1 year ago

Thanks for the merge! 🎉