You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by davebshow <gi...@git.apache.org> on 2017/11/01 23:49:40 UTC
[GitHub] tinkerpop pull request #741: TINKERPOP-1807 Gremlin-Python doesn't support G...
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 <da...@gmail.com>
Date: 2017-11-01T23:31:10Z
Implemented support for missing core GraphSON types in gremlin-python
----
---
[GitHub] tinkerpop pull request #741: TINKERPOP-1807 Gremlin-Python doesn't support G...
Posted by davebshow <gi...@git.apache.org>.
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 issue #741: TINKERPOP-1807 Gremlin-Python doesn't support GraphSON...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/741
All tests pass with `docker/build.sh -t -n -i`
VOTE +1
---
[GitHub] tinkerpop pull request #741: TINKERPOP-1807 Gremlin-Python doesn't support G...
Posted by FlorianHockmann <gi...@git.apache.org>.
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...
Posted by davebshow <gi...@git.apache.org>.
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...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/tinkerpop/pull/741
---
[GitHub] tinkerpop issue #741: TINKERPOP-1807 Gremlin-Python doesn't support GraphSON...
Posted by FlorianHockmann <gi...@git.apache.org>.
Github user FlorianHockmann commented on the issue:
https://github.com/apache/tinkerpop/pull/741
LGTM, VOTE: +1
---
[GitHub] tinkerpop issue #741: TINKERPOP-1807 Gremlin-Python doesn't support GraphSON...
Posted by robertdale <gi...@git.apache.org>.
Github user robertdale commented on the issue:
https://github.com/apache/tinkerpop/pull/741
VOTE +1
---
[GitHub] tinkerpop issue #741: TINKERPOP-1807 Gremlin-Python doesn't support GraphSON...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/741
Please include a CHANGELOG entry as well as user upgrade documentation.
---