Conversation
|
A follow-up (two months later ...).
I don't see why we need to concern ourselves with anything on the original |
| @@ -1,7 +1,5 @@ | |||
| pyodbc | |||
| six | |||
| sqlalchemy | |||
There was a problem hiding this comment.
Why is sqlalchemy being removed here? Is it being handled as a requirement/dependency somewhere else?
There was a problem hiding this comment.
Probably, but since we do import it directly in odm2api I guess it is better to leave it untouched.
We never know if the other dependencies that brings it will drop it in the future.
requirements.txt
Outdated
| six | ||
| sqlalchemy | ||
| #-e git+https://github.com/ODM2/[email protected]#egg=geoalchemy-0.7.4 | ||
| #shapely |
There was a problem hiding this comment.
Might as well also remove the commented-out shapely. It's pointless or totally optional w/o geoalchemy.
|
@ocefpaf I'll wait to hear back from you on the two line comments I've just made, before trying to merge. |
I addressed both comments in the last commit. Should be OK now. Thanks! |
|
Great, thanks. Merging now. |
2 participants
This PR partially addresses #59 (comment)
Note that PR #58 (mentioned in #59) was merged but geoalchemy/geoalchemy#42 was not. It seems that if the path is to remove
geoalchemygeoalchemy/geoalchemy#42 will need to be re-written or just closed.I could not find any other code reference to
geoalchemyin master. I am unsure of the status on the multiple branches. BTW that many branches put a really high bar to get third party contributions to the code. I would be nice to reduce the development tomasterand eliminate the branches to temporary work on active PRs or stable releases.I could not get the CIs to pass but looking at the history it seems that it is not passing for sometime now.
cc @emiliom and @valentinedwv
xref #59