Skip to content

Use sqlalchemy outerjoin for compability reasons - #192

Closed
mkofahl wants to merge 1 commit into
geopython:masterfrom
faegi:sqlalchemycompability
Closed

Use sqlalchemy outerjoin for compability reasons#192
mkofahl wants to merge 1 commit into
geopython:masterfrom
faegi:sqlalchemycompability

Conversation

@mkofahl

@mkofahl mkofahl commented Mar 13, 2018

Copy link
Copy Markdown
Contributor

Just a suggestion: allow use of older sqlalchemy versions by using outerjoin() instead of join(.., isouter=True). In some environments python modules may have to be used from the distro, so packages may be slightly out-of-date. I recognized the requirements.txt, but for now this fix allows me to keep on running this software.

@justb4

justb4 commented Mar 13, 2018

Copy link
Copy Markdown
Member

@mkofahl hard to assess impact of this PR. Code was recently added via PR #187, maybe @cezio can review and give green light? Apart from that, we would like to "pin" requirements.txt, as to have reproducible installs via issue #138.

@cezio

cezio commented Mar 13, 2018

Copy link
Copy Markdown
Contributor

AFAIR, I used .join(isouter=True) there, because .outerjoin() won't work with sqlite properly. But i didn't check this thoroughly, so if this code works fine with sqlite as well, i'm ok.

@mkofahl

mkofahl commented Mar 13, 2018

Copy link
Copy Markdown
Contributor Author

Tested with sqlite3 3.8.7.1-1+deb8u2 in terms it's not crashing. It's definitly not a very forward-looking PR. So I'm fine not merging it, but maybe it helps someone else having trouble with GHC and an can only join an iterable error.

@justb4

justb4 commented Mar 19, 2019

Copy link
Copy Markdown
Member

Closing: has been open for over a year, please open issue if relevant.

@justb4 justb4 closed this Mar 19, 2019
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.

3 participants