Enum fixes (typing and inheritance)

[chriswhite199]

Addresses issues #96 & #164

[chriswhite199]

This is more-or-less a duplicate of #24, but adds in a test to assert the DDL for postgres and sqlite.

[carlbordum]

This is a good patch - hope it gets merged :rocket:

[tofuurem]

why not merged yet?

[dane-skipper]

+1 for merging @chriswhite199 really small nit, but it may be worth combining the three sqlalchemy import statements you have into a single one

[mattyoungberg]

Getting the SQLAlchemy native enum supported here is important is making or breaking whether or not I use it in my project (as well as many others, it sounds like)... I'd make a PR myself, but this one is perfectly fine (As well as #24 except this has tests). This might be overkill, but I'm gonna ping all the maintainers because something like this should NOT be left for 6 months without action on a library that's meant to nicely compliment FastAPI:

@tiangolo @egrim @sobolevn @Batalex @Lehoczky @yaquelinehoyos @sebastianmarines @robcxyz @leynier @nomagick @hanxiao

Please, address this. @tiangolo was able to oversee 3 releases of FastAPI last week alone. This library needs more love.

[Batalex]

Hi,

This might be overkill

It is, considering that tiangolo aside, you pinged one-time contributors, not maintainers.

May I suggest that you do the following to follow proper OSS etiquette?

• Raise your concerns about the maintenance of the library in a dedicated issue. You have some valid points, but the substance and the form are equally important when raising such concerns.
• Ease the burden of maintenance. If you feel like the PR is good, your code review, or you testing the branch in a toy project will be greatly appreciated by the maintainer(s).

edit: typos

[antont]

making or breaking whether or not I use it in my project (as well as many others, it sounds like)... I'd make a PR myself

You can also just merge the PR to your fork. That's what we do to use SQLModel, just merge PRs from here to our organizational fork and install that in our dev envs and deployment. These changes are not so big, and if get synched to main later and there's more work there, I don't see a problem really.

That's kind of the point of Git, the main project maintainers don't block work and collaboration by others, even if are busy.

[mattyoungberg]

@Batalex, my apologies for pinging contributors.

Raise your concerns about the maintenance of the library in a dedicated issue. You have some valid points, but the substance and the form are equally important when raising such concerns.

I mean, I COULD, but just a scan of the last thirty days of issues shows barely, if any, maintainer activity. Raising an issue might be "form" but it gets the library nowhere if the issues aren't being engaged with in the first place. Pinging all maintainers is kind of a last resort to get some traction before I consider this library deadlocked from lack of maintenance.

Heck, traction could be adding some maintainers that DO have capacity to work on this library; I'm certainly not faulting maintainers for having a life. But spending a half hour to message some people and see if they want to be maintainers could go a long ways for the 142 issues and 71 PRs that are currently open. It's only going to snowball.

Ease the burden of maintenance. If you feel like the PR is good, your code review, or you testing the branch in a toy project will be greatly appreciated by the maintainer(s).

This is a good suggestion, and I'll remember it for the future. I'm going to be using the content of these commits anyway in a forked version of this library, but I wish I didn't have to. I'll report back how well it works.

@antont

You can also just merge the PR to your fork. That's what we do to use SQLModel, just merge PRs from here to our organizational fork and install that in our dev envs and deployment.

My previous comment reveals that this was my plan because I wasn't expecting a resolution right away, but it does mean another repository gets added to my company's VCS, which isn't optimal. But alas, I don't think I have another option.

I'm more frustrated with the fact that this library was released three times in a spurt in August, then twice in a spurt in December; there has never been a steady cadence of releases or of merging of good PRs. This library gets advertised as a great compliment to FastAPI and has great docs that suggests its a polished project (because @tiangolo is good at that), only to learn that it's not getting any kind of TLC at all.

The SQLAlchemy native enum has existed since version 1.1 of SQLAlchemy in 2016, which means that when SQLModel was built in 2021, there were things that were missed. Naturally, that's the course of software; you get something out first and then firm things up over time. But to release and not put infrastructure in place to allow those improvements to happen is frustrating to a developer like me who sees potential in this library.

[mattyoungberg]

Third PR regarding this is #197

[codecov[bot]]

Codecov Report

Merging #165 (c011523) into main (2fab481) will increase coverage by 0.02%. The diff coverage is 100.00%.

:exclamation: Current head c011523 differs from pull request most recent head 37c1eb7. Consider uploading reports for the commit 37c1eb7 to get more accurate results

@@            Coverage Diff             @@
##             main     #165      +/-   ##
==========================================
+ Coverage   97.59%   97.61%   +0.02%     
==========================================
  Files         182      183       +1     
  Lines        6060     6072      +12     
==========================================
+ Hits         5914     5927      +13     
+ Misses        146      145       -1     

Impacted Files	Coverage Δ
sqlmodel/main.py	84.54% <100.00%> (+0.62%)	:arrow_up:
tests/test_enums.py	100.00% <100.00%> (ø)
sqlmodel/sql/expression.py	87.17% <0.00%> (-10.26%)	:arrow_down:
tests/test_missing_type.py	92.30% <0.00%> (-1.03%)	:arrow_down:
tests/conftest.py	100.00% <0.00%> (ø)
docs_src/tutorial/fastapi/teams/tutorial001.py	100.00% <0.00%> (ø)
docs_src/tutorial/fastapi/delete/tutorial001.py	100.00% <0.00%> (ø)
docs_src/tutorial/fastapi/update/tutorial001.py	100.00% <0.00%> (ø)
docs_src/tutorial/fastapi/read_one/tutorial001.py	100.00% <0.00%> (ø)
..._src/tutorial/fastapi/relationships/tutorial001.py	100.00% <0.00%> (ø)
... and 19 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[github-actions[bot]]

📝 Docs preview for commit 57a2b68521df266ffa5aab0a1a8df555dbe5aa58 at: https://630a9d68bb46d54c5f6e264d--sqlmodel.netlify.app

[tiangolo]

Awesome, thank you @chriswhite199! In particular, thanks for writing thorough tests! :bow:

Thanks everyone for the discussion here. :coffee:

If any of you wants to help maintain SQLModel, the best help I can get is helping others with their questions in their issues. That's what really takes most of the time. Thanks! :cake:

This fix will be available in the next release, SQLModel 0.0.7, in the next hours. :tada:

[github-actions[bot]]

📝 Docs preview for commit c01152379ef0f0b255cf87f2e1071ef8e1f8f83a at: https://630a9e50b736cd5383509bb7--sqlmodel.netlify.app

[github-actions[bot]]

📝 Docs preview for commit 37c1eb738003c23450e2fce86cb8e1b6826238a0 at: https://630a9f0b94f02852b60aa30d--sqlmodel.netlify.app