Add extra 'citext' to add support for CIText and CIText arrays on PostgreSQL database #125
Conversation
|
This should be mentioned in the docs or otherwise people won't know about the extra. |
|
@agronholm good point, thank you. I went ahead and added it |
sqlacodegen/codegen.py
Outdated
| # Support CIText and CIText[] in PostgreSQL via sqlalchemy-citext | ||
| try: | ||
| from citext import CIText # noqa: F401 | ||
| except (NameError, ImportError): |
There was a problem hiding this comment.
This should be moved to after the Geoalchemy2 import.
Under what circumstances can this raise NameError?
There was a problem hiding this comment.
Oops, that was supposed to be ModuleNotFoundError not NameError. Though I realize now that's overly specific and will fail on Python2 anyway as it doesn't have ModuleNotFoundError- which is a subclass of ImportError. Will remove that and commit, nice catch
There was a problem hiding this comment.
(fixing the order as well)
sqlacodegen/codegen.py
Outdated
| @@ -19,6 +19,12 @@ | |||
| from sqlalchemy.types import Boolean, String | |||
| from sqlalchemy.util import OrderedDict | |||
|
|
|||
| # Support CIText and CIText[] in PostgreSQL via sqlalchemy-citext | |||
| try: | |||
| from citext import CIText # noqa: F401 | |||
There was a problem hiding this comment.
Is it not enough to import the module?
There was a problem hiding this comment.
Should be no different since it's not directly used. I'll change that to only import the module
- No need to import the CIText class, the module will do - Changed import order of citext per @agronholm
|
Pushed those changes up. Sorry 'bout that, especially the one that would have broken Python2 (!!!) |
|
Thanks! |
After entering #124, I took a quick look at this and saw that it was pretty trivial to add. I added it as an optional / extra so it can be installed using
pip install sqlacodegen[citext]without impacting users that don't want or need itIs this something you'll take upstream?
Thanks!