[GitHub] tinkerpop pull request #741: TINKERPOP-1807 Gremlin-Python doesn't support G...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/tinkerpop/pull/741


---

[GitHub] tinkerpop pull request #741: TINKERPOP-1807 Gremlin-Python doesn't support G...

[davebshow]

Github user davebshow commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/741#discussion_r150620490
  
--- Diff: 
gremlin-python/src/main/jython/tests/structure/io/test_graphson.py ---
@@ -121,6 +125,19 @@ class X(object):
 serdes.objectify.assert_called_once_with(value, reader)
 assert o is serdes.objectify()
 
+def test_datetime(self):
+dt = self.graphson_reader.readObject(json.dumps({"@type": 
"g:Date", "@value": 1481750076295}))
+assert isinstance(dt, datetime.datetime)
--- End diff --

Done


---

[GitHub] tinkerpop pull request #741: TINKERPOP-1807 Gremlin-Python doesn't support G...

[davebshow]

Github user davebshow commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/741#discussion_r150589566
  
--- Diff: 
gremlin-python/src/main/jython/tests/structure/io/test_graphson.py ---
@@ -121,6 +125,19 @@ class X(object):
 serdes.objectify.assert_called_once_with(value, reader)
 assert o is serdes.objectify()
 
+def test_datetime(self):
+dt = self.graphson_reader.readObject(json.dumps({"@type": 
"g:Date", "@value": 1481750076295}))
+assert isinstance(dt, datetime.datetime)
--- End diff --

Yeah I guess. TBH, I don't find any of these tests too meaningful, which is 
why I added the functional IO tests at the end that actually send and receive 
messages from the server and verify that they are equal. I can add the asserts 
though if you want.


---

[GitHub] tinkerpop pull request #741: TINKERPOP-1807 Gremlin-Python doesn't support G...

[FlorianHockmann]

Github user FlorianHockmann commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/741#discussion_r150529077
  
--- Diff: 
gremlin-python/src/main/jython/tests/structure/io/test_graphson.py ---
@@ -121,6 +125,19 @@ class X(object):
 serdes.objectify.assert_called_once_with(value, reader)
 assert o is serdes.objectify()
 
+def test_datetime(self):
+dt = self.graphson_reader.readObject(json.dumps({"@type": 
"g:Date", "@value": 1481750076295}))
+assert isinstance(dt, datetime.datetime)
--- End diff --

Wouldn't it make sense to also assert that the value is correct here? Then 
the test would validate the GraphSON reader completely and it stays consistent 
with the other tests in this class.


---

[GitHub] tinkerpop pull request #741: TINKERPOP-1807 Gremlin-Python doesn't support G...

[davebshow]

GitHub user davebshow opened a pull request:

https://github.com/apache/tinkerpop/pull/741

TINKERPOP-1807 Gremlin-Python doesn't support GraphSON types g:Date, 
g:Timestamp and g:UUID

https://issues.apache.org/jira/browse/TINKERPOP-1807

This PR add support for GraphSON types `UUID`, `Date`, and `Timestamp` as 
well as associated tests. `UUID` and `Date` are straightforward, as the Python 
standard library includes `Date` (`datetime.datetime`) and `UUID` (`uuid.UUID`) 
objects. However, in Python, a timestamp is just a float, so in order to 
support serialization to a GraphSON type Timestamp, I added a dummy class 
`timestamp` to the `statics` module, similar to the `long` class for Python 3.

Note, another PR will be required for master with implementations for both 
GraphSON2 and 3. I will open upon approval of this one.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/apache/tinkerpop TINKERPOP-1807

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/tinkerpop/pull/741.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #741


commit 3b651ff58cdf210a63a0101d0a311c7076de2b0e
Author: davebshow 
Date:   2017-11-01T23:31:10Z

Implemented support for missing core GraphSON types in gremlin-python




---