Skip to content
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

geoalchemy vs geoalchemy2 dependency #13

Closed
emiliom opened this issue Nov 30, 2015 · 16 comments
Closed

geoalchemy vs geoalchemy2 dependency #13

emiliom opened this issue Nov 30, 2015 · 16 comments

Comments

@emiliom
Copy link
Member

emiliom commented Nov 30, 2015

@sreeder and @horsburgh: I'm creating this issue to help us keep track of the status.

Stephanie, were you able to make progress on this last week?

@sreeder
Copy link
Contributor

sreeder commented Dec 4, 2015

This issue has been completed in the geometry branch and has been merged into the setup branch

@emiliom emiliom mentioned this issue Dec 4, 2015
@emiliom
Copy link
Member Author

emiliom commented Dec 4, 2015

@sreeder, one question: when we talked on the phone I think you said that using geoalchemy would involve making some changes to it. Was that the case, or am I mixing things up? If it is true, it means we'll have to maintain our own fork of geoalchemy in the ODM2 github organization. Not a problem per se, just wanted to make that clear.

@sreeder
Copy link
Contributor

sreeder commented Dec 5, 2015

@emiliom Yes, there is one classname that has to be changed. I have made a note of it in the requirements.txt file. But I have been planning on keeping our own copy in the repository, thanks for the reminder.

@emiliom
Copy link
Member Author

emiliom commented Dec 7, 2015

This is just for reference:

@emiliom
Copy link
Member Author

emiliom commented Dec 7, 2015

I've forked geoalchemy into the ODM2 org: https://github.com/ODM2/geoalchemy. Then I created a branch (odm2) and made Stephanie's single-line edit there. Then I tested by first installing this fork/branch via pip install git+https://github.com/ODM2/geoalchemy.git@odm2, then installing odm2api (setup_em branch) via pip install git+https://github.com/ODM2/ODM2PythonAPI.git@setup_em.

So far so good! import odm2api and import geoalchemy didn't complain.

I don't know how to specify a github repo, rather than the pypi default, as the source of a dependency requirement; so I didn't change the requirements/setup files in odm2api. Hopefully @sreeder can take care of that, specially once we promote my geoalchemy odm2 branch to master (I was just being extra careful).

@emiliom
Copy link
Member Author

emiliom commented Dec 7, 2015

Things to follow up on, probably as separate issues:

  • Is odm2api.ODM2.apiCustomType no longer used anywhere? I assume so, b/c it has a geoalchemy2 import statement, so that would throw an error. If this is the case, it should be removed (or better yet, an experimental geoalchemy2 branch should be created and used as a future reference/archive). But I wonder if the now missing functionality from that module is related to the possible problems/unexpected behavior I've run into.
  • In odm2api.ODM2.models, the use of from geoalchemy import * should be changed so geoalchemy is imported with a namespace. Importing everything makes debugging more difficult, and may lead to unexpected conflicts.
  • SamplingFeatures.FeatureGeometry is currently hardwired (in odm2api.ODM2.models) to support only Point feature types. This will need to be addressed, but that can happen in follow-up development, say in a geometry branch.

Also see my related, latest comment in issue 11.

@sreeder
Copy link
Contributor

sreeder commented Dec 8, 2015

@emiliom
you are correct, the apiCustomType is no longer being used.
I will work on changing over the import statement and see if there is a simple way to expand the support of feature types beyond Point. I'm not sure the best way to address the shaply issue at the moment but I may be able to write a custom repr function that will print it out as WKT.

sreeder pushed a commit that referenced this issue Dec 9, 2015
@valentinedwv
Copy link
Member

I changed the dependencies in setup.py and requirements.txt.
On Windows you may/will need to add git to your path. for me it was:
C:\Program Files (x86)\Git\cmd

@emiliom
Copy link
Member Author

emiliom commented Dec 16, 2015

Thanks, @valentinedwv ! Have you tested this? I tried installing, letting pip install git+https://github.com/ODM2/ODM2PythonAPI.git handle the geoalchemy dependency. Instead of installing odm2/geoalchemy.git@odm2 (labelled as version 0.7.3dev), it installed geoalchemy from pypi (0.7.2).

I don't know enough about pip packaging to figure out what's wrong.

@emiliom
Copy link
Member Author

emiliom commented Dec 16, 2015

Just more confirmation that the setup dependency requirement for geoalchemy is not configured correctly. My odm2api tests ran just fine when I pre-installed geoalchemy via pip install, like this:

pip install git+https://github.com/ODM2/geoalchemy.git@odm2
pip install git+https://github.com/ODM2/ODM2PythonAPI.git

@valentinedwv
Copy link
Member

Didn't hard core check it, but when I ran it in pycharm, it complained about not pulling from git...
Checking now pycharm shows 0.7.3.dev0
As for setup... getting whacked with shapley.

Added >= to geoalchemy to see if that forces it.

@emiliom
Copy link
Member Author

emiliom commented Dec 17, 2015 via email

@emiliom
Copy link
Member Author

emiliom commented Dec 23, 2015

Has anyone tested Dave's latest change to the geoalchemy dependency specification? ie, is the right geoalchemy version installed when pip installing odm2api?

@horsburgh
Copy link
Member

@emiliom - Not sure I'm on track here, but to test the API (which I have been doing a little over the past couple of days), I used these instructions from Stephanie to get a Python environment named "sdl" set up using conda:

conda create -n sdl sqlalchemy pymysql psycopg2 pyodbc shapely pandas pyyaml ipython-notebook
source activate sdl
pip install git+https://github.com/ODM2/Geoalchemy.git@master
pip install git+https://github.com/ODM2/ODM2PythonAPI.git@master

Then, I had to manually change line 242 in the file:

/anaconda/envs/sdl/lib/python2.7/site-packages/geoalchemy/base.py 

From: class SpatialComparator(ColumnProperty.ColumnComparator):
To: class SpatialComparator(ColumnProperty.Comparator):

It seems like if we are maintaining our own copy of Geoalchemy and I'm using pip to install, I shouldn't have to do this last step???

@emiliom
Copy link
Member Author

emiliom commented Dec 23, 2015

It seems like if we are maintaining our own copy of Geoalchemy and I'm using pip to install, I shouldn't have to do this last step???

You're right. And that hasn't been needed for over two weeks. The right installation statements are:

pip install git+https://github.com/ODM2/geoalchemy.git@odm2
pip install git+https://github.com/ODM2/ODM2PythonAPI.git
  • No need to specify @master on odm2api itself
  • Currently the change on our geoalchemy fork is on the odm2 branch. We could easily merge it to master; I just wanted to let others (@sreeder? you?) have a chance to comment first.

What @valentinedwv was/is working on was correctly building in the geoalchemy dependency specification for odm2api, so that pip installing odm2api will take care of geoalchemy directly, w/o the user needing to explicitly install geoalchemy.

@emiliom
Copy link
Member Author

emiliom commented Jan 13, 2016

Closing this issue. I've created instructions for installing odm2api using conda packages for all the dependencies except geoalchemy, followed by a single pip install command to install odm2api and geoalchemy. See README.md.

Gory packaging details can be found on issue #18 and associated pull requests (PR #19 and PR #20).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants