Skip to content

🐛 Fix Enum Type Mapping #24

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

Closed
wants to merge 3 commits into from
Closed

🐛 Fix Enum Type Mapping #24

wants to merge 3 commits into from

Conversation

zhangbc97
Copy link

According to SQLAlchemy's Generic Type Document

Python's class inherit from enum.Enum should be wrapped with sqlalchemy.sql.Enum(sqlalchemy.sql.sqltypes.Enum) when it is set to sqlalchemy.Column()'s type_ field.

Change SQLAlchemyEnum to _Enum
@creatorrr
Copy link

Is there anything else pending for this to be merged, @tiangolo ?

RobertRosca added a commit to RobertRosca/sqlmodel that referenced this pull request Nov 2, 2021
@invokermain
Copy link

It's worrying that this isn't merged in yet or had any feedback in 6 months, why does @tiangolo create these libraries without adequate maintainer resource to support them? (Suffering from success probably) ... Is it not possible to find community maintainers? Can I be a community maintainer? I'll merge it in! I'll even write some pretty release notes 🎨

@Miguelme
Copy link

Should this branch get rebased and try to get merged from maintainers?

Copy link

@oldfielj-ansto oldfielj-ansto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@oldfielj-ansto
Copy link

@tiangolo SQLModel might not be a primary focus for you right now, that's understandable, but might I suggest you look at establishing a trusted maintainer team, so we can push some of these fixes through.

synodriver pushed a commit to synodriver/sqlmodel that referenced this pull request Jun 24, 2022
@tiangolo tiangolo changed the title Fix Enum Type Mapping 🐛 Fix Enum Type Mapping Aug 27, 2022
@codecov
Copy link

codecov bot commented Aug 27, 2022

Codecov Report

Merging #24 (e915864) into main (2fab481) will decrease coverage by 0.07%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main      #24      +/-   ##
==========================================
- Coverage   97.59%   97.51%   -0.08%     
==========================================
  Files         182      182              
  Lines        6060     6031      -29     
==========================================
- Hits         5914     5881      -33     
- Misses        146      150       +4     
Impacted Files Coverage Δ
sqlmodel/main.py 83.96% <50.00%> (+0.04%) ⬆️
sqlmodel/sql/expression.py 87.17% <0.00%> (-10.26%) ⬇️
tests/test_missing_type.py 92.30% <0.00%> (-1.03%) ⬇️
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%> (ø)
...src/tutorial/fastapi/response_model/tutorial001.py 100.00% <0.00%> (ø)
... and 16 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
Copy link

📝 Docs preview for commit e915864 at: https://630a9c0d8dc0eb53d76d5dbb--sqlmodel.netlify.app

@tiangolo
Copy link
Member

Thank you @zbclove! 🍰

And thanks everyone for the discussion. ☕

If you want to help me, the best help I can get is helping others with their questions in issues. That's what consumes most of the time.

This was solved in #165 that included tests, so I'll close this one. The fix will be available in the next release, SQLModel 0.0.7, in the next hours. 🎉

Thanks for the effort! ☕

@tiangolo tiangolo closed this Aug 27, 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

Successfully merging this pull request may close these issues.

6 participants