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

fix: get_attr_data doesn't handle default factory correctly #1134

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

bzczb
Copy link
Contributor

@bzczb bzczb commented Oct 22, 2023

Motivation

get_attr_data() didn't check default values for "factory" types, which need to be called in order to get the real default value, so several use cases that worked with dataclasses were broken for attrs classes.

Test Plan

New fixture parameter added for "factory" variant of the tests/structured_conf/data/ attrs classes, similar to the post-Python 3.11 dataclass tests.
New test added for error on takes_self parameter to factory (does not exist in dataclasses).

Fixes

Fixes #945 get_attr_data doesn't handle default factory correctly

Copy link
Collaborator

@Jasha10 Jasha10 left a comment

Choose a reason for hiding this comment

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

Thanks @bzczb.

@michaelvay
Copy link

Hi,
Is there an expected release date for this PR?

@odelalleau
Copy link
Collaborator

Hi, Is there an expected release date for this PR?

No, there's no ETA yet for the next Hydra release.

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.

get_attr_data doesn't handle default factory correctly
4 participants