spatial icon indicating copy to clipboard operation
spatial copied to clipboard

Compatibility with neo4j v5.x

Open seabamirum opened this issue 2 years ago • 3 comments

This pull request includes the changes necessary for this project to compile with Neo4j 5, including updates to logging and the getCoordinate() method, which now returns an array instead of a list. I've tested at least spatial.withinDistance in my application and it appears to be functioning as expected, although I haven't tried to run any of the official tests.

Please note that I inadvertently ran the "organize imports" feature in my IDE, which led to some unnecessary changes in other files. I apologize for any confusion this may cause.

Let me know if you have any questions or need further information. Thank you for your consideration of this pull request.

seabamirum avatar Mar 09 '23 22:03 seabamirum

Hi @seabamirum, have you signed the CLA? Please read the information at https://neo4j.com/developer/cla/ and send the email to Neo4j.

In the meantime, I am travelling and can only look at the PR the week after next. However, I think it is a good idea that you run the tests and fix any failing tests because we cannot merge the PR without all tests passing.

craigtaverner avatar Mar 10 '23 08:03 craigtaverner

I made a few more adjustments, and the only test that fails now is this one:

org.opentest4j.AssertionFailedError: expected: <POLYGON ((0 0, 0 5, 2 5, 2 6, 4 6, 4 10, 10 10, 10 4, 6 4, 6 2, 5 2, 5 0, 0 0))> but was: <POLYGON ((2 5, 2 6, 4 6, 4 10, 10 10, 10 4, 6 4, 6 2, 5 2, 5 0, 0 0, 0 5, 2 5))>
	at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
	at org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1124)
	at org.neo4j.gis.spatial.pipes.GeoPipesDocTest.unite_all(GeoPipesDocTest.java:672)

Perhaps this is a red herring and the order of vertices has changed in neo4j v5, but I don't know enough to be sure.

seabamirum avatar Mar 11 '23 21:03 seabamirum

  1. add_node_point_layer_and_search_multiple_points_precision fails with

java.lang.AssertionError: Expected two nodes when using high precision Expected: <2L> but: was <1L>

  1. import_osm_and_polygons_withinDistance fails with

java.lang.AssertionError: Result should have 'node' expected:<{node=Node[388], distance=0.0, osmNode=Node[387], props={lon=12.9772464, version=2, lat=56.0569953, node_osm_id=2938842290, timestamp=1462219905000}}> but was:<map containing ["node"->ANYTHING]>

seabamirum avatar May 06 '23 02:05 seabamirum

What else is required for this to be merged? I would love to see 5.x support included :)

ajmeese7 avatar Feb 21 '24 17:02 ajmeese7

There was another PR that was merged that also has 5.X support, so unfortunately now I'm just maintaining this branch separately. Maybe some time I'll figure out what all the conflicts are and resolve them.

seabamirum avatar Feb 22 '24 00:02 seabamirum