spmallette commented on a change in pull request #1280:
URL: https://github.com/apache/tinkerpop/pull/1280#discussion_r411321142
##########
File path: gremlin-python/src/main/jython/tests/driver/test_client.py
##########
@@ -129,6 +129,17 @@ def test_multi_conn_pool(client):
assert len(result_set.all().result()) == 6
+def test_client_bytecode_with_int(client):
+ g = Graph().traversal()
+ t = g.V().has('age', 851401972585122).count()
Review comment:
Thus far we've forced python users to be explicit and think in java
terms, thus:
```python
g.V().has('age', long(851401972585122)).count()
```
but I suppose it's better if this were just handled automatically as it
might yield less confusion and be more pythonic. The test you added here is
fine but please add a test here:
https://github.com/apache/tinkerpop/blob/3.3-dev/gremlin-python/src/main/jython/tests/structure/io/test_graphsonV2d0.py#L306
and here:
https://github.com/apache/tinkerpop/blob/3.3-dev/gremlin-python/src/main/jython/tests/structure/io/test_graphsonV3d0.py#L357
to more directly test GraphSON. Please do not remove the existing tests that
use the old `long()` syntax as I think it's still worth testing that since it's
something we will continue to support. Just add new assertions for your new
logic. I guess it wouldn't hurt to add tests to cover both positive and
negative values in your range.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
