Open emiliom opened 8 years ago
The errors listed above (on this issue) would suggest that something may have changed with SQLAlchemy in January; and that fixing this problem may require more SQLAlchemy Fu than what I have.
But it turns out that I still had a conda environment built before the bad changes happened, and I can successfully run my SQLite-testing IPython notebook with that environment. I'm no longer able to trace when exactly I built that env, but it must have been between January 10 & 20. By looking at all differences in package (dependency) versions between that env and an env built today, I recreated a conda env with exactly the same package (dependency) versions as the old one, except for odm2api
proper. I did this to isolate the problem to the odm2api package itself. The SQLite error with ReadODM2.getSamplingFeature
methods is still there and is identical. So, the problem is definitely with the the changes we've made, not with recent changes in SQLAlchemy or any other dependency.
(Note that I made a couple of edits to this comment after I first created it).
I've overhauled models.SamplingFeatures
to handle FeatureGeometry
consistently across databases and provide helpful results. The new code is in the branch sfgeometry_em_1, for testing. I've tested it with PostgreSQL and SQLite, but I expect it to work with MySQL and MSSQL as well.
I wanted to change FeatureGeometry
reading to return a shapely.geometry
object (eg, Point), but I don't understand SQLAlchemy well enough to do that. Instead, FeatureGeometry
is returned as a somewhat database-specific encoding as handled by geoalchemy1. I added a new SamplingFeatures
method, shape
, that returns a shapely object representation of FeatureGeometry
; I also overhauled the __repr__
method to return a WKT representation of FeatureGeometry
regardless of the actual encoding. BTW, I don't know if adding a method to the SamplingFeatures "Base" object will have negative consequences elsewhere. So far I haven't seen any.
So, for a SamplingFeatures object sf
:
sf.FeatureGeometry
is a geoalchemy1
object that's somewhat database-specific. It's not user friendly, so avoid using it directly!sf.shape()
is a consistent shapely object with interfaces and properties that are very useful and well known across the Python geospatial ecosystem. Use this!sf.shape().wkt
returns a WKT string representation (eg, "POINT (-55.2 3.1)")@cdesyoun, can you give it a try? Remember, it's not on master, but on the sfgeometry_em_1
branch.
@Iluvatar, this addresses the problems I described over email. Give it a try with the sample ODM2 SQLite database you have. Let us know if you see problems with my implementation.
@cdesyoun, have you had a chance to test this new SamplingFeatures code from the sfgeometry_em_1 branch? If so, does it work for your needs?
Anyone else? It'd be great to have testing and confirmation on MySQL and MS SQL Server, too ...
I want to decide if we can merge it into master soon, say, next week.
@emiliom I did pull up sfgeometry_em_1 branch and tested it. When looking into your new changed SamplingFeatures codes in models.py, you did not override "Geometry" type itself to return WKT string, like apiCustomType.py did. In your change, this function, "def repr(self):" was performed to get WKT string for FeatureGeometry column, which means ODM2 REST api does not matter with this. Still get WKB object because of reading the value of "FeatureGeometry" column. As I mentioned before, for this, I added one function below to return WKT string:
def getWKTFromWKB(self,value):
if value:
binary = binascii.unhexlify(str(value))
point = wkb.loads(binary)
point = '{0}'.format(wkt.dumps(point))
return point
else:
return None
So, currently, ODM2 REST api does not have any issues for ODM2 API. Please go ahead to merge it into master.
Thanks for testing, @cdesyoun. You're correct that I'm not overriding the Geometry type. Instead, I provided a new convenience method on SamplingFeatures that will return a WKT from the FeatureGeometry column. For SamplingFeature sf
, you can get the WKT string like this: sf.shape().wkt
. That way you don't have to define and maintain such a conversion function yourself. In addition, be aware that your function will probably only work for PostgreSQL/PostGIS, b/c other database systems don't return a hex WKB like PostGIS does.
Would it be more helpful if the shape
method is defined as a stand-alone function that can be applied to a value derived from FeatureGeometry?
@emiliom I have focused on overriding only Geometry type. When getting this email, I realized that you put the "shape()" function into SamplingFeature object to return WKT string. Thank you so much to reminding me. When using "sf.shape().wkt" if FeatureGeometry is not null, it worked perfect in ODM2REST API. I updated all of my codes and pushed them into github. Also, I updated testing site. Now, there are no any issues in this case.
@emiliom In addition, in your code,
def shape(self):
"""
Method name based on shapely shapely.geometry.shape() function.
Returns a shapely geometry object
:return geomshape:
"""
_FeatureGeometry = self.FeatureGeometry
if _FeatureGeometry is not None:
if is_hex(_FeatureGeometry.geom_wkb):
# to parse wkb hex string directly
geomshape = wkb.loads(_FeatureGeometry.geom_wkb, hex=True)
# _FeatureGeometry = GeometryColumn('featuregeometry', Geometry)
else:
geomshape = wkt.loads(str(_FeatureGeometry.geom_wkb))
return geomshape
When FeatureGemetry type is null and calling this function, local variable, "geomshape" error was occurred. I think you need to declare it as global variable before this statement, "if _FeatureGeometry is not None:".
@cdesyoun, I'm really glad the shape()
function works for you! Thanks also for pointing out the bug when FeatureGeometry is null; you're right, it's a bug. I'll fix this soon, hopefully tonight, and will push the fix to the sfgeometry_em_1
branch. I'll think about merging that branch into master around Friday.
I've addressed the bug where null FeatureGeometry was not handled. The fix is in the sfgeometry_em_1
branch.
@emiliom Great! Thanks.
In #24 we went back forth with some changes to ODM2/models.py and possibly ODM2/services/readService.py regarding SamplingFeatures and the use of geoalchemy 1. A couple of merges were involved. These changes sort-of continued in #27, and have some linkage earlier to #13.
Before that, we got Examples/Sample.py to work top to bottom with its SQLite sample database (FYI, I never tested the SamplingFeature creation code). Here's a working IPython notebook version I made on 1/21
Now, I'm unable to access
ReadODM2.getSamplingFeature
methods to work anymore. They work on PostgreSQL, but not SQLite. With @horsburgh's ODM2 Little Bear River sqlite sample file, a method request such asgetSamplingFeaturesByType('Site')
leads to this error:Note the problem with the
AsBinary
SQL function. Probing a bit further, I see the error comes fromsqlalchemy/engine/default.py
, specifically line 450 below:I've gone through the relevant commit history for ODM2/models.py and ODM2/services/readService.py since early December, and can't find anything that's different from a time in mid January when things were working fine. I'm stumped.
@denvaar has moved to greener pastures and @sreeder is still away on maternity leave. So, for now I'm creating this issue as documentation of the problem. I'll keep tackling it, and will add more information in a bit.