Repository navigation
✨ Add support for Literal types - #1439
nsaccente wants to merge 31 commits into
Conversation
Sorry, something went wrong.
|
I have no idea how to add the proper labels to this PR, it looks like the permissions aren't set up to allow me to. |
Sorry, something went wrong.
This comment was marked as outdated.
|
@nsaccente, thanks for working on this! |
Sorry, something went wrong.
Sure. I'll try to get that done sometime this week. |
Sorry, something went wrong.
…del into bugfix-57/literal-satype
|
I've been fighting with getting the tests to work. I've been able to get things to pass in some versions of python, but not others. Any guidance on how to run these CI tests locally? |
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
svlandeg
left a comment
There was a problem hiding this comment.
I gave this some more thought and agree that this feature is more complex that it initially seemed. Just avoiding the crash when using Literal isn't sufficient: we also want to map to the right datatype ànd somehow respect the value restriction that the Literal definition presents.
Building on the changes by Yurii to identify all-bool or all-int columns, I've now also pushed another update that creates SQL constraints for the values in the Literal. This way, the data can't get corrupted and the user won't receive values that don't match the expected Literal definition.
Having mixed types in a Literal is still not ideal. I wonder whether we should support it by just making it all str (as the PR does now), whether we should reject it entirely (by raising an exception) or whether there is some other way we could deal with this 🤔
@YuriiMotov: would love a review from you on this!
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
|
Follow-up PR as this one has been closed (despite being actively worked on): #1736 |
Sorry, something went wrong.
This PR addresses issue 57. The original issue explains that attempting to use a Literal type annotation in a SQLModel raises a type error from issubclass. This is because Literal is not a class, but a typing._SpecialForm, so static funcs like isinstance and issubclass don't work with it.
The fix was pretty straightforward, I just added a check before an isinstance or issubclass would have been called that checks if type_ is Literal.