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

Allow variadic tuples of length 0. Fixes #104 #105

Merged
merged 1 commit into from
Jan 29, 2024
Merged

Allow variadic tuples of length 0. Fixes #104 #105

merged 1 commit into from
Jan 29, 2024

Conversation

intentionally-left-nil
Copy link
Contributor

With this patch, the following code works:

from dataclasses import dataclass
from dataclass_wizard import fromdict
@dataclass
class Test:
    my_ints: tuple[int, ...]


fromdict(Test, {"my_ints": []})

I am not sure why the required count should ever not be zero, based on my understanding of the code? If you have ... it means zero or more, not one or more.

The new unit test I added failed before this patch, and succeeds afterwards. None of the existing tests fail, so maybe this is a safe fix? Otherwise could you please expand on the logic & we can figure out the appropriate solution.

Fixes #104

# Check for the count of parsers which don't handle `NoneType` - this
# should exclude the parsers for `Union` types that have `None` in the
# list of args.
self.required_count = 0 if None in self.first_elem_parser[0] else 1
Copy link
Owner

Choose a reason for hiding this comment

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

I don't actually remember what this line was actually doing 😅

But I'm glad you caught it, that this is where we need to make the change. Totally right!

@rnag
Copy link
Owner

rnag commented Jan 29, 2024

LGTM @intentionally-left-nil ! I kicked off the CI builds, but will go ahead and merge in either case, as it seems like a minor change to me.

Update: Error when running TC's for Python 3.8, so I'll have to look into that when I have time. Will go ahead and merge now and work on patch release with the changes soon. Thanks again for opening this PR!

@rnag rnag merged commit 3d2ca55 into rnag:main Jan 29, 2024
4 of 5 checks passed
@rnag
Copy link
Owner

rnag commented Jan 29, 2024

@intentionally-left-nil This should be fixed as of v0.22.3. Please pull in latest version of lib to test and let me know.

Also I credited your contribution in the changelog, hope you don't mind 🙂.

@intentionally-left-nil
Copy link
Contributor Author

Cheers!

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.

Loading a Variadic Tuple fails for length 0
2 participants