FEAT: Add ST_START_POINT and ST_END_POINT to PostGIS backend #2081
Conversation
ian-r-rose
force-pushed
the
start-end-points
branch
3 times, most recently
from
d0922bf
to
0c327a0
Compare
|
@ian-r-rose maybe the tests errors are related to the different approach of the 2 backends. IIRC OmniSciDB, for example, doesn't recognize that a geometry contains a point that touch the border ... and PostGreSQL recognize it. So maybe the data tests need to be changed to some data that don't touch the border. |
|
but not sure yet if it is the case ... I would need to investigate that |
|
Yeah, it seems it is some issue with the way the two backends handle
boundaries. I'll take a closer look.
…On Thu, Feb 20, 2020, 7:33 AM Ivan Ogasawara ***@***.***> wrote:
but not sure yet if it is the case ... I would need to investigate that
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2081?email_source=notifications&email_token=ABLWQN7RXJUO5XFG4TWIQSDRD2PDLA5CNFSM4KUZF4U2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMOWWHY#issuecomment-589130527>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABLWQN6L27QZGR2M777WFTLRD2PDLANCNFSM4KUZF4UQ>
.
|
ian-r-rose
force-pushed
the
start-end-points
branch
2 times, most recently
from
b81c7c1
to
101e53a
Compare
|
@ian-r-rose I am talking to the OmniSci team to check this issue. |
|
Thanks @xmnlab! |
xmnlab
changed the title
|
@xmnlab, @ian-r-rose is this still active? If it is, can you please add a release note, and merge master? |
|
Hi @datapythonista, sorry, missed your note. Yes, I can update this PR. |
ian-r-rose
force-pushed
the
start-end-points
branch
from
101e53a
to
4456ade
Compare
|
Thanks @ian-r-rose. Do you mind adding a release note in https://github.com/ibis-project/ibis/blob/master/docs/source/release/index.rst please? And also merge master again? We've got some problems with the CI that should be fixed now. Thanks! |
|
@ian-r-rose do you have time to add the release note and merge master? |
|
@datapythonista I'm remembering why this stalled in the first place -- there are some apparent differences in how the omniscidb geo functionality works and how postgis works with respect to boundary points. I was initially unable to find a test that worked for both of them. Probably the best thing to do is to have separate tests for those two backends. |
|
If the difference is small, and it's just in the cases where the point is in the boundary, I think we can have a single test with an In any case, that doesn't sound as a major blocker, I think should be easy to implement. Can you take care of it? Thanks! |
|
@ian-r-rose do you still want to move forward with this? |
|
@datapythonista I'm interested in getting this wrapped up, but I must say, I am having a really tough time getting the new test suite setup working. Is it documented anywhere? |
Good point. Things didn't change much, and there is still some refactoring pending, but you should be able to run tests for postgres with |
ian-r-rose
force-pushed
the
start-end-points
branch
from
4456ade
to
037d9c4
Compare
|
Hello @ian-r-rose! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2020-09-03 14:02:10 UTC |
consider boundaries.
ian-r-rose
force-pushed
the
start-end-points
branch
from
849d52a
to
efd7ad5
Compare
|
Thanks @datapythonista and @jreback All green except for one skipped backend that seems to be unrelated. |
|
thanks @ian-r-rose |
This adds a couple of missing geospatial operations to the PostGIS backend.