Skip to content

opt_frozen_dataclass: more improvements - #274

Closed
matthiasdiener wants to merge 3 commits into
inducer:opt-frozen-hashablefrom
matthiasdiener:opt-frozen-improvements
Closed

opt_frozen_dataclass: more improvements#274
matthiasdiener wants to merge 3 commits into
inducer:opt-frozen-hashablefrom
matthiasdiener:opt-frozen-improvements

Conversation

@matthiasdiener

@matthiasdiener matthiasdiener commented Dec 3, 2024

Copy link
Copy Markdown
Contributor
  • raise an error when specifying frozen
  • add tests
  • add support for specifying weakref_slot

targets #273.

inducer and others added 2 commits December 3, 2024 10:49
@matthiasdiener
matthiasdiener force-pushed the opt-frozen-improvements branch from a11975b to a24ad7c Compare December 3, 2024 18:29
@matthiasdiener

Copy link
Copy Markdown
Contributor Author

What do you think @inducer?

@matthiasdiener
matthiasdiener marked this pull request as ready for review December 3, 2024 20:14
Comment thread pytools/__init__.py
Comment on lines +3025 to +3027
# Make it possible to override 'frozen' in the class definition for testing.
# It would be nice to have something like https://discuss.python.org/t/allow-debug-to-be-set-at-runtime/64840
loc_frozen = __debug__ if "_frozen_override" not in cls.__dict__ else False

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Could you explain the use case?

@matthiasdiener matthiasdiener Dec 4, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's just meant for testing so that we can "fake" setting __debug__ = False.

Comment thread pytools/__init__.py

# Make it possible to override 'frozen' in the class definition for testing.
# It would be nice to have something like https://discuss.python.org/t/allow-debug-to-be-set-at-runtime/64840
loc_frozen = __debug__ if "_frozen_override" not in cls.__dict__ else False

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

frozen_override could be in kwargs maybe?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think that's a good idea.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I was thinking this needed tests. Thanks for adding them!

Comment thread pytools/__init__.py
slots: bool = False,
# Added in 3.11.
# weakref_slot: bool = False
**kwargs: Any, # Extra, version dependent arguments (weakref_slot in 3.11)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Is a kwargs-only ParamSpec a thing?

@inducer
inducer force-pushed the opt-frozen-hashable branch 2 times, most recently from cb2683a to d22dec6 Compare December 4, 2024 17:47
@inducer

inducer commented Dec 4, 2024

Copy link
Copy Markdown
Owner

Thanks! I've rolled most of this into #273, with attribution. We can tackle the override later on if it's still needed.

@inducer
inducer force-pushed the opt-frozen-hashable branch from d22dec6 to 7b4d68a Compare December 4, 2024 17:52
@inducer
inducer deleted the branch inducer:opt-frozen-hashable December 4, 2024 18:01
@inducer inducer closed this Dec 4, 2024
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