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

Decorate Pack as dataclass #3215

Merged
merged 9 commits into from
Mar 12, 2025
Merged

Conversation

HalfWhitt
Copy link
Contributor

@HalfWhitt HalfWhitt commented Feb 26, 2025

With the aliases all defined as class attribute descriptors as of #3213, Pack can be successfully decorated as a dataclass.

The only test that fails-as-is is test_mixin.py::test_constructor, because the auto-genereated dataclass __init__ throws a TypeError rather than a NameError when an erroneous property name is provided. I've changed what the test looks for, and also added a try/except to BaseStyle's fallback __init__ so that the same error is generated under Python 3.9.

Edit: Huh. What are the odds, it's been exactly one year, to the day, since I started this with beeware/travertino#141. I definitely didn't anticipate how long that would take 😄

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@HalfWhitt
Copy link
Contributor Author

HalfWhitt commented Mar 8, 2025

Decorating Pack as a dataclass attempts to overrides the default <toga.style.pack.Pack object at [memory address]> representation with Pack(name=value, ...). Unfortunately, it tries to do so for all properties, including ones that haven't been set, and aliases which aren't currently valid (which raises an exception). So I've added a BaseStyle.__repr__ that behaves very similarly to the existing __str__, only listing properties that are set. I've also added repr=False to the dataclass arguments to avoid overriding it.

(While I was next to it, I also tweaked __str__ by replacing the generator expression in join() with a list comp.)
Edit: Reverted this change to be consistent with #3213. Also makes it fewer lines in this case. I also made the list-comp-to-generator switch for the new __repr__ method.)

@HalfWhitt HalfWhitt marked this pull request as ready for review March 12, 2025 01:28
@HalfWhitt
Copy link
Contributor Author

It's gonna be so satisfying to rip out all this backwards compatibility code once we no longer support Toga < 0.5 and Python 3.9.

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

Looks good - a fairly minor set of changes at the end of the day. Jeez, why did it take over 12 months to complete such a small PR? 😝 🤣

And yes, the "FINALLY WE GET TO DEPRECATE 3.X" purge is always really satisfying.

@freakboy3742 freakboy3742 merged commit 0d7254a into beeware:main Mar 12, 2025
48 checks passed
@freakboy3742
Copy link
Member

I think that was your final blocker for a Toga 0.5 release? Have I missed anything?

@HalfWhitt
Copy link
Contributor Author

Looks good - a fairly minor set of changes at the end of the day. Jeez, why did it take over 12 months to complete such a small PR? 😝 🤣

Oh, you 🤣

I think that was your final blocker for a Toga 0.5 release? Have I missed anything?

That should finally be it! Let 'er rip!

@HalfWhitt HalfWhitt deleted the pack-dataclass branch March 12, 2025 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants