Make all transformed data classes frozen by default

[kkom]

Description

This addresses #3396 – haven't ported the rationale to the PR description just yet.

If the general approach is accepted, things to do before shipping:

• Decide if this needs new tests
• See if any of the failing tests are caused by it
• Update the documentation
• Update the release notes

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Documentation

Issues Fixed or Closed by This PR

#3396

Checklist

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[sourcery-ai]

PR Type: Enhancement

PR Summary: The pull request introduces a significant change to the default behavior of data classes transformed by the @dataclass_transform decorator in the Strawberry GraphQL framework. By setting frozen_default=True, it makes all instances of the decorated classes immutable by default. This change aims to address the issue raised in the linked GitHub issue, enhancing the predictability and safety of data handling within the framework.

Decision: Comment

📝 Type: 'Enhancement' - not supported yet.

• Sourcery currently only approves 'Typo fix' PRs.

✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.

No details provided.

✅ Small diff: the diff is small enough to approve with confidence.

No details provided.

General suggestions:

• Given the broad impact of making transformed data classes immutable by default, it's crucial to ensure comprehensive testing around this change. Consider adding more specific tests that verify the immutability of instances and how it interacts with the framework's core functionalities.
• Documenting this change is essential, as it alters the fundamental behavior of data classes. Update the documentation to highlight the new default behavior, providing examples of how it affects data class usage and any necessary steps for users who may need to opt-out of this behavior.
• Engage with the community by sharing this change in relevant discussion forums or channels. Gathering early feedback from users can provide valuable insights into potential edge cases or usage scenarios that were not initially considered.

Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨

Share Sourcery

• X
• Mastodon
• LinkedIn
• Facebook

---

Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

[botberry]

Hi, thanks for contributing to Strawberry 🍓!

We noticed that this PR is missing a RELEASE.md file. We use that to automatically do releases here on GitHub and, most importantly, to PyPI!

So as soon as this PR is merged, a release will be made 🚀.

Here's an example of RELEASE.md:

Release type: patch

Description of the changes, ideally with some examples, if adding a new feature.

Release type can be one of patch, minor or major. We use semver, so make sure to pick the appropriate type. If in doubt feel free to ask :)

Here's the tweet text:

🆕 Release (next) is out! Thanks to @Komorowski_K for the PR 👏

Get it here 👉 https://beta.strawberry.rocks/release/(next)

[botberry]

Hi, thanks for contributing to Strawberry 🍓!

We noticed that this PR is missing a RELEASE.md file. We use that to automatically do releases here on GitHub and, most importantly, to PyPI!

So as soon as this PR is merged, a release will be made 🚀.

Here's an example of RELEASE.md:

Release type: patch

Description of the changes, ideally with some examples, if adding a new feature.

Release type can be one of patch, minor or major. We use semver, so make sure to pick the appropriate type. If in doubt feel free to ask :)

Here's the tweet text:

🆕 Release (next) is out! Thanks to @Komorowski_K for the PR 👏

Get it here 👉 https://beta.strawberry.rocks/release/(next)

[codecov]

Codecov Report

Merging #3397 (87305e1) into main (be9a2d5) will decrease coverage by 1.06%.
The diff coverage is n/a.

❗ Current head 87305e1 differs from pull request most recent head 953398c. Consider uploading reports for the commit 953398c to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3397      +/-   ##
==========================================
- Coverage   96.41%   95.35%   -1.06%     
==========================================
  Files         498      498              
  Lines       31133    31131       -2     
  Branches     3814     3809       -5     
==========================================
- Hits        30016    29685     -331     
- Misses        911     1237     +326     
- Partials      206      209       +3     

[codspeed-hq]

CodSpeed Performance Report

Merging #3397 will degrade performances by 29.15%

_{Comparing kkom:02-25-Make_all_transformed_data_classes_frozen_by_default (87305e1) with main (be9a2d5)}

Summary

❌ 1 regressions
✅ 12 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	main	kkom:02-25-Make_all_transformed_data_classes_frozen_by_default	Change
❌	test_execute_basic	9.8 ms	13.9 ms	-29.15%

[kkom]

The performance hit is somewhat expected sadly... https://rednafi.com/python/statically_enforcing_frozen_dataclasses/

But I didn't expect it to be that bad :(

[kkom]

From googling around, I see an option here: https://threeofwands.com/attra-iv-zero-overhead-frozen-attrs-classes/ But I'm not sure if it would work in this case.

[kkom]

Seeing the performance hit, I'm pretty sure this shouldn't be shipped. So I'm closing the PR.

However, I may raise the point of having a static-only data class immutability configuration setting in the Python typing forum (as per @erictraut's suggestion here: microsoft/pyright#7334 (comment) )

[kkom]

Posted on the Python typing forum here: https://discuss.python.org/t/static-only-frozen-data-classes-or-other-ways-to-avoid-runtime-overhead/46968

[patrick91]

@kkom thanks for reporting all the findings! please keep updated if you find some solution 😊

[kkom]

The thread I started suggested a promising solution using "if TYPE_CHECKING". Will put a new PR trying it out at some point in the next few days. :)