Skip to content

Conversation

danarmak
Copy link

@danarmak danarmak commented Sep 2, 2025

This extends the fix in #650 to the tagged union strategy. I reported the issue in #682.

Without this, using include_subclasses with tagged_union and a generic parent class raises the error AttributeError: __subclasses__. Did you mean: '__subclasscheck__'?.

Note: I don't know whether to edit default_tag_generator to say return (typing.get_origin(typ) or typ).__name__. It doesn't break any tests, but I'm not sure if there are cases where it wouldn't be appropriate, or what else ought to be changed if this is. For now I left it as a tag_generator override at the callsite, but it would be better for users not to have to pass this. Please advise.

The tests don't pass for me locally on the main branch (regardless of my change), but the failure is in a test that doesn't use these strategies and all the tests passed when I rebased my fix on 25.1.1, so I hope I'm just confused about something unrelated. (The failure is tests/test_gen_dict.py::test_individual_overrides - assert 'Factory' not in "{'a': ('', ... '_b': None}")

(ETA: the tests now pass, see comments)

@danarmak danarmak force-pushed the fix-generics-with-tagged-union branch from 60feae8 to af4565d Compare September 2, 2025 18:44
@Tinche
Copy link
Member

Tinche commented Sep 2, 2025

Thanks, I'll take a look when I get some free cycles. Can you get me a more detailed error message on that flaky test in the meantime?

@danarmak
Copy link
Author

danarmak commented Sep 3, 2025

Here is the failing test output.

@Tinche
Copy link
Member

Tinche commented Sep 3, 2025

Cool, thanks. Can you rebase, I reworked those tests.

@danarmak danarmak force-pushed the fix-generics-with-tagged-union branch from af4565d to a198b33 Compare September 3, 2025 14:35
@danarmak
Copy link
Author

danarmak commented Sep 3, 2025

I rebased. All tests now pass.

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