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

Breaks marshmallow_dataclass.NewType #91

Closed
gabor-akeero opened this issue Aug 19, 2022 · 4 comments
Closed

Breaks marshmallow_dataclass.NewType #91

gabor-akeero opened this issue Aug 19, 2022 · 4 comments

Comments

@gabor-akeero
Copy link

gabor-akeero commented Aug 19, 2022

In is_new_type there is an explicit check for the module name being either typing or typing_extensions, so types created via marshmallow_dataclass.NewType are no longer recognised, and therefore cannot be used in marshmallow dataclasses.

This check on __module__ makes it impossible to define and use any custom NewType implementations, even if they are intended to be accepted as NewType.

As the pre-3.10 implementations of typing.NewType define only two extra attributes __name__ and __supertype__, I'd suggest a check on these two (besides __qualname__), and on their types:

... isinstance(getattr(tp, '__supertype__', None), type) and isinstance(getattr(tp, '__name__'), str) and ...

This would check everything that we can be sure of, and wouldn't block the custom NewType-implementations.

@gabor-akeero gabor-akeero changed the title breaks marshmallow_dataclass.NewType Breaks marshmallow_dataclass.NewType Aug 19, 2022
@vit-zikmund
Copy link

vit-zikmund commented Aug 23, 2022

@gabor-akeero it's questionable whether this needs to be fixed here (although an optional is_new_type parameter for a list of external modules that would also match could be nice). As marshmallow_dataclass.NewType virtually uses the exact pre 3.10 typing.NewType peppered with a couple more attributes, I think a nicer solution might be to use the typing.NewType and monkeypatch the resulting object, which doesn't change the original intention but makes it more independent on whatever happens in the typing (or a tightly related) module.

@vit-zikmund
Copy link

FYI, my fix for marshmallow_dataclass has been cherry-picked into lovasoa/marshmallow_dataclass#211 and merged to master. Unless reverted, the fix fill will be available with any next release (which I have no control of, tho).

@dairiki
Copy link

dairiki commented Oct 4, 2022

FYI, my fix for marshmallow_dataclass has been cherry-picked into lovasoa/marshmallow_dataclass#211 and merged to master. Unless reverted, the fix fill will be available with any next release (which I have no control of, tho).

A release has just been made of marshmallow-dataclass 8.5.9 that includes @vit-zikmund’s fixes for marshmallow_dataclass.NewType.

@gabor-akeero
Copy link
Author

This issue has been fixed at marshmallow_dataclass

@gabor-akeero gabor-akeero closed this as not planned Won't fix, can't repro, duplicate, stale Nov 16, 2022
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

No branches or pull requests

3 participants