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

SamplingFeature read failures with SQLite #29

Open
emiliom opened this issue Feb 2, 2016 · 10 comments
Open

SamplingFeature read failures with SQLite #29

emiliom opened this issue Feb 2, 2016 · 10 comments

Comments

@emiliom
Copy link
Member

emiliom commented Feb 2, 2016

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 as getSamplingFeaturesByType('Site') leads to this error:

(sqlite3.OperationalError) no such function: AsBinary [SQL: u'SELECT 
samplingfeatures.samplingfeatureid AS odm2_samplingfeatures_samplingfeatureid, 
samplingfeatures.samplingfeatureuuid AS odm2_samplingfeatures_samplingfeatureuuid, 
samplingfeatures.samplingfeaturetypecv AS odm2_samplingfeatures_samplingfeaturetypecv, 
samplingfeatures.samplingfeaturecode AS odm2_samplingfeatures_samplingfeaturecode, 
samplingfeatures.samplingfeaturename AS odm2_samplingfeatures_samplingfeaturename, samplingfeatures.samplingfeaturedescription AS odm2_samplingfeatures_samplingfeaturedescription, 
samplingfeatures.samplingfeaturegeotypecv AS odm2_samplingfeatures_samplingfeaturegeotypecv, 
samplingfeatures.elevation_m AS odm2_samplingfeatures_elevation_m, 
samplingfeatures.elevationdatumcv AS odm2_samplingfeatures_elevationdatumcv, 
AsBinary(samplingfeatures.featuregeometry) AS odm2_samplingfeatures_featuregeometry
\nFROM samplingfeatures 
\nWHERE samplingfeatures.samplingfeaturetypecv = ?'] [parameters: ('site',)]

Note the problem with the AsBinary SQL function. Probing a bit further, I see the error comes from sqlalchemy/engine/default.py, specifically line 450 below:

449     def do_execute(self, cursor, statement, parameters, context=None):
450         cursor.execute(statement, parameters)

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.

@emiliom
Copy link
Member Author

emiliom commented Feb 2, 2016

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.

@emiliom
Copy link
Member Author

emiliom commented Feb 7, 2016

(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.

@emiliom
Copy link
Member Author

emiliom commented Feb 10, 2016

@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.

@cdesyoun
Copy link
Contributor

@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.

@emiliom
Copy link
Member Author

emiliom commented Feb 17, 2016

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?

@cdesyoun
Copy link
Contributor

@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.

@cdesyoun
Copy link
Contributor

@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:".

@emiliom
Copy link
Member Author

emiliom commented Feb 17, 2016

@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.

@emiliom
Copy link
Member Author

emiliom commented Feb 18, 2016

I've addressed the bug where null FeatureGeometry was not handled. The fix is in the sfgeometry_em_1 branch.

@cdesyoun
Copy link
Contributor

@emiliom Great! Thanks.

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

No branches or pull requests

2 participants