-
Notifications
You must be signed in to change notification settings - Fork 4
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
Unstructuring IntegerSchema
broken
#62
Comments
https://github.com/Tinche/uapi/blob/main/src/uapi/openapi.py#L63 I understand why Literal kind of makes sense here, since you only want a singular value of the enum, I am beginning to think its not really the intended semantics. IIUC E.g. instead of The "proper" name then would be something like In any case since it is an attrs attribute I think it would make more sense here to have a validator that only allows a specific value. |
I can fix the current behavior in cattrs (and will), but let me ask you why is this a problem? If you have an openapi schema, you generally want to dump it to json so it can be served. This will work properly since the json module will coerce that enum value into the string "integer". Just unstructuring the data (but not dumping it to json) is a niche use case. I'd say if you want to manipulate the schema in python, it's much better to manipulate it before unstructuring. |
That explains the intended usage of that :) Partially I just wasn't sure what to expect and since string types got serialized into strings I thought maybe the integers etc. should be as well. Further than that its just a matter of modularity, reusability,avoiding unnecessarily serialization roundtrips, and generally the same ethos of cattrs of not having arbitrary decisions made for you. I'll admit I don't have a specific "this blocks X" use case, but I'd object to it being a niche use case. Perhaps from the perspective what uapi needs it for it is. This is probably a discussion better held in the context of this functionality as its own package/module though. Happy to expand more on what I'm doing with schemas and how it fits in. Briefly it includes code generation and general configuration validation, not just serving OpenAPI schemas. |
I agree with the consistency argument - we should fix cattrs.Converter to handle this consistently. I will fix this for the next version. However, in my view the preconf converters are and should be free to customize behaviors like this in the name of performance and compatibility. For example, in some cases the new msgspec converter will avoid unstructuring attrs classes if it detects msgspec will unstrcture them identically, because msgspec can do it faster. Similarly the orjson converter will leave datetimes to orjson itself by default. The ability to do this is pretty cool to me. Maybe we should document this somewhere explicitly though. |
I very much agree. It would be great to have optimized marshalling that leverage types. I guess my only issue then was that in this module there was only the one "converter" that was only intended for marshalling and I was looking for one that did (un)structuring. So again not an issue for this code base really, and more for the still hypothetical one that does only schema interop. A use case that popped up: In the |
Alright, that's an interesting use case. We might make the converter an optional parameter in the future? In the mean time, you can probably dump the schema straight to JSON and load it back up with a json library - that will probably get you the format you need? |
Yes and not that bad since I only do it once. |
Root cause I believe is: python-attrs/cattrs#321
Repro:
The text was updated successfully, but these errors were encountered: