You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by David Brown <da...@gmail.com> on 2016/06/15 18:27:28 UTC
gremlin_python GLV
Hello everyone,
I was reading through the gremlin_python GLV code this morning, and
overall it looks like it should work pretty smoothly. Thanks for doing
this Marko, really cool work! I just wanted to comment on a few things
that popped out at me on my first reading.The major issue I see is
that it is not currently Python 2/3 compatible. This is due to two
things:
1. The way iterators are implemented in Python 2/3 is different. This
problem could be remedied by adding a method to
``PythonGraphTraversal``, something like:
def __next__(self):
return self.next()
then, in the ``next`` method, changing line 113 to
``next(self.results)``, should take care of this problem.
2. The ``GroovyTranslator`` class checks for type ``long``, this no
longer exists in Python 3. Determining what to submit as a Long might
require some discussion, but this could be easily fixed by adding
something like:
import sys
if sys.version_info.major > 2:
long = int
to the top of the file.
Other than this, there are some minor details that could be cleaned
up. Particularly:
1. Using ``isinstance`` instead of ``type`` to perform type checking.
This will recognize subclasses and is the most Pythonic way to do
this.
2. Formatting - indents, and line spacing. Typically, Python methods
are separated by a single line, and indents use four spaces. This is
really just cosmetic for readability.
3. CamelCase vs. underscores. I understand that to emulate Gremlin,
the traversal methods etc. should use CamelCase. But I wonder if the
helper classes (Translators) should use the Python convention of using
underscores to name methods. Python class names use camel case by
convention.
Finally, the implementation of the B class may need some work, but
we'll have to play around with it a bit to figure out how the best
approach to doing this.
I'm sure there are more improvements, but I just wanted to get a
conversation going. I would be happy to make a PR with some of these
changes.
Also, gremlinclient will soon support the RemoteConnection interface,
I'll send out a link to the docs once I get everything up and running.
Thanks again to everyone at TInkerpop for all the hard work!
Best,
Dave
--
David M. Brown
R.A. CulturePlex Lab, Western University
Re: gremlin_python GLV
Posted by David Brown <da...@gmail.com>.
By the error I expect you have the server configured for REST.
gremlinclient still only supports websockets.
On Wed, Jun 15, 2016 at 8:10 PM, David Brown <da...@gmail.com> wrote:
> Are u using websocket or http?
>
> On Jun 15, 2016 17:09, "Marko Rodriguez" <ok...@gmail.com> wrote:
>>
>> Dar.
>>
>> >>> g.V().name.toList()
>> Traceback (most recent call last):
>> File "<stdin>", line 1, in <module>
>> File
>> "/Users/marko/software/tinkerpop/gremlin-variant/src/main/jython/gremlin_python/gremlin_python.py",
>> line 107, in toList
>> return list(iter(self))
>> File
>> "/Users/marko/software/tinkerpop/gremlin-variant/src/main/jython/gremlin_python/gremlin_python.py",
>> line 110, in next
>> self.results =
>> self.remote_connection.submit(self.translator.script_engine,
>> self.translator.traversal_script, self.bindings)
>> File
>> "/Library/Python/2.7/site-packages/gremlinclient/tornado_client/remote_connection.py",
>> line 20, in submit
>> results = self._loop.run_sync(lambda:
>> File "/Library/Python/2.7/site-packages/tornado/ioloop.py", line 453, in
>> run_sync
>> return future_cell[0].result()
>> File "/Library/Python/2.7/site-packages/tornado/concurrent.py", line
>> 232, in result
>> raise_exc_info(self._exc_info)
>> File "/Library/Python/2.7/site-packages/tornado/gen.py", line 1014, in
>> run
>> yielded = self.gen.throw(*exc_info)
>> File
>> "/Library/Python/2.7/site-packages/gremlinclient/tornado_client/remote_connection.py",
>> line 27, in _execute
>> conn = yield self._pool.acquire()
>> File "/Library/Python/2.7/site-packages/tornado/gen.py", line 1008, in
>> run
>> value = future.result()
>> File "/Library/Python/2.7/site-packages/tornado/concurrent.py", line
>> 232, in result
>> raise_exc_info(self._exc_info)
>> File "<string>", line 3, in raise_exc_info
>> tornado.httpclient.HTTPError: HTTP 400: Bad Request
>>
>>
>>
>>
>> > On Jun 15, 2016, at 5:36 PM, David Brown <da...@gmail.com> wrote:
>> >
>> > Nice Leif!
>> >
>> > I just added very basic support for the RemoteConnection interface in
>> > gremlinclient:
>> >
>> >
>> > http://gremlinclient.readthedocs.io/en/latest/usage.html#the-remoteconnection-object
>> >
>> > It needs more tests etc., but it seems to be working well. If you want
>> > to try it out, you can pip install gremlinclient from github:
>> >
>> > pip install git+https://github.com/davebshow/gremlinclient.git
>> >
>> > More tomorrow...
>> >
>> > On Wed, Jun 15, 2016 at 7:17 PM, Leifur Halldor Asgeirsson
>> > <LA...@zerofail.com> wrote:
>> >> Thanks! I'll submit my PR in the morning.
>> >>
>> >> ________________________________________
>> >> From: Marko Rodriguez <ok...@gmail.com>
>> >> Sent: June 15, 2016 6:13 PM
>> >> To: dev@tinkerpop.apache.org
>> >> Subject: Re: gremlin_python GLV
>> >>
>> >> Hi,
>> >>
>> >> That is a really good idea.
>> >>
>> >> Want to do a PR to the branch or do you want me to just take your
>> >> notes/gist and make it happen?
>> >>
>> >> Marko.
>> >>
>> >> http://markorodriguez.com
>> >>
>> >>
>> >>
>> >>> On Jun 15, 2016, at 3:55 PM, Leifur Halldor Asgeirsson
>> >>> <LA...@zerofail.com> wrote:
>> >>>
>> >>> Hi all,
>> >>> I have a couple of suggestions for the GLV.
>> >>>
>> >>> Firstly, it would be useful to be able to inject arbitrary
>> >>> expressions, such as static constructors. For example, in an application
>> >>> that I'm currently working on, I use Titan's Geoshape property type. I would
>> >>> like to be able to call one of the static constructors on the Geoshape class
>> >>> in a script, passing it bound parameters. Something like
>> >>>
>> >>> "g.V().has('location', geoWithin(Geoshape.circle(lat, lon, radius)))"
>> >>> with bound parameters {'lat': 45, 'lon': 45, 'radius': '10'}
>> >>>
>> >>> I took an initial stab at this today, and this is what I came up with:
>> >>> https://gist.github.com/leifurhauks/5a843379183123dbed1134a92691a335
>> >>>
>> >>> With those changes, I can use that static constructor in a traversal
>> >>> as follows:
>> >>>
>> >>>>>> translator = GroovyTranslator('g')
>> >>>>>> g = PythonGraphTraversalSource(translator)
>> >>>>>> t = g.V().has('location', RawExpression('Geoshape.point(', B('lat',
>> >>>>>> 45), ', ', B('lon', 45), ')'))
>> >>>>>> str(t)
>> >>> 'g.V().has("location", Geoshape.point(lat, lon))'
>> >>>>>> t.bindings
>> >>> {'lon': 45, 'lat': 45}
>> >>>
>> >>> That raw expression isn't very readable, but a simple helper class can
>> >>> fix that:
>> >>>
>> >>> class Geoshape(object):
>> >>> @staticmethod
>> >>> def point(latitude, longitude, symbols=('lat', 'lon')):
>> >>> return RawExpression(
>> >>> 'Geoshape.point(', B(symbols[0], latitude), ', ',
>> >>> B(symbols[1], longitude), ')')
>> >>>
>> >>> Now I can rewrite the previous traversal to be much clearer:
>> >>>
>> >>> t = g.V().has('location', Geoshape.point(45, 45))
>> >>>
>> >>>
>> >>> If this seems like a reasonable approach, I would be happy to submit a
>> >>> PR.
>> >>>
>> >>>
>> >>> I have one other suggestion, but this one is tiny. Because most of the
>> >>> steps on PythonGraphTraversal have the same implementation, it would be
>> >>> possible to specify that implementation once in a function factory, like
>> >>> this:
>> >>>
>> >>> def simple_step(name):
>> >>> def step_method(self, *args):
>> >>> self.translator.addStep(self, name, *args)
>> >>> for arg in args:
>> >>> if type(arg) is B:
>> >>> self.bindings[arg.symbol] = arg.value
>> >>> return self
>> >>> return step_method
>> >>>
>> >>> Then, on PythonGraphTraversal, all the step methods that use that
>> >>> implementation could be declared like this:
>> >>>
>> >>> class PythonGraphTraversal(object):
>> >>> def __init__(self, translator, remote_connection=None):
>> >>> # elided ...
>> >>> # top methods elided
>> >>> has = simple_step('has')
>> >>> hasId = simple_step('hasId')
>> >>> hasKey = smple_step('hasKey')
>> >>> # and so on...
>> >>>
>> >>>
>> >>> ________________________________________
>> >>> From: David Brown <da...@gmail.com>
>> >>> Sent: June 15, 2016 3:23 PM
>> >>> To: dev@tinkerpop.apache.org
>> >>> Subject: Re: gremlin_python GLV
>> >>>
>> >>> Ok Stephen I will. I can't claim to be an expert (especially when it
>> >>> comes to the Java ecosystem), but I will definitely take a look.
>> >>>
>> >>> Also, Marko, Python doesn't really have primitives as such. The built
>> >>> in function isinstance should work for everything in gremlin_python
>> >>> e.g. ``isinstance(var, bool)``. I can take a look at this when I make
>> >>> a PR if you'd like, but it really isn't a huge deal anyway.
>> >>>
>> >>> On Wed, Jun 15, 2016 at 3:16 PM, Stephen Mallette
>> >>> <sp...@gmail.com> wrote:
>> >>>> David, it would also be great to get your feedback on the
>> >>>> packaging/deployment approach we have so far. I have some basic
>> >>>> figured out
>> >>>> for deployment to pypi over maven via twine, but it could use an
>> >>>> experts
>> >>>> eye. You probably don't need to check that part out now as you are
>> >>>> already
>> >>>> have some other stuff to look at, but I just wanted to mention that
>> >>>> so that
>> >>>> it was in your mind for later. Thanks for your help on this.
>> >>>>
>> >>>> On Wed, Jun 15, 2016 at 3:02 PM, Marko Rodriguez
>> >>>> <ok...@gmail.com>
>> >>>> wrote:
>> >>>>
>> >>>>> Hello,
>> >>>>>
>> >>>>>> I was reading through the gremlin_python GLV code this morning, and
>> >>>>>> overall it looks like it should work pretty smoothly. Thanks for
>> >>>>>> doing
>> >>>>>> this Marko, really cool work! I just wanted to comment on a few
>> >>>>>> things
>> >>>>>> that popped out at me on my first reading.The major issue I see is
>> >>>>>> that it is not currently Python 2/3 compatible. This is due to two
>> >>>>>> things:
>> >>>>>
>> >>>>> Cool! Thank you for taking your time to review the work.
>> >>>>>
>> >>>>>> 1. The way iterators are implemented in Python 2/3 is different.
>> >>>>>> This
>> >>>>>> problem could be remedied by adding a method to
>> >>>>>> ``PythonGraphTraversal``, something like:
>> >>>>>>
>> >>>>>> def __next__(self):
>> >>>>>> return self.next()
>> >>>>>>
>> >>>>>> then, in the ``next`` method, changing line 113 to
>> >>>>>> ``next(self.results)``, should take care of this problem.
>> >>>>>
>> >>>>> Updated.
>> >>>>>
>> >>>>>> 2. The ``GroovyTranslator`` class checks for type ``long``, this no
>> >>>>>> longer exists in Python 3. Determining what to submit as a Long
>> >>>>>> might
>> >>>>>> require some discussion, but this could be easily fixed by adding
>> >>>>>> something like:
>> >>>>>>
>> >>>>>> import sys
>> >>>>>> if sys.version_info.major > 2:
>> >>>>>> long = int
>> >>>>>>
>> >>>>>> to the top of the file.
>> >>>>>
>> >>>>> Updated.
>> >>>>>
>> >>>>>>
>> >>>>>> Other than this, there are some minor details that could be cleaned
>> >>>>>> up. Particularly:
>> >>>>>>
>> >>>>>> 1. Using ``isinstance`` instead of ``type`` to perform type
>> >>>>>> checking.
>> >>>>>> This will recognize subclasses and is the most Pythonic way to do
>> >>>>>> this.
>> >>>>>
>> >>>>> Seems isinstance() isn’t a method on primitives — only objects.
>> >>>>> Thus, the
>> >>>>> code got complex. Left it with type().
>> >>>>>
>> >>>>>>
>> >>>>>> 2. Formatting - indents, and line spacing. Typically, Python
>> >>>>>> methods
>> >>>>>> are separated by a single line, and indents use four spaces. This
>> >>>>>> is
>> >>>>>> really just cosmetic for readability.
>> >>>>>
>> >>>>> Uh. The problem is that the source code is auto-generated from
>> >>>>> GremlinPythonGenerator. If you want to tweak, please do so:
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>> https://github.com/apache/tinkerpop/blob/TINKERPOP-1278/gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy
>> >>>>> <
>> >>>>>
>> >>>>> https://github.com/apache/tinkerpop/blob/TINKERPOP-1278/gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy
>> >>>>>>
>> >>>>>
>> >>>>>
>> >>>>>> 3. CamelCase vs. underscores. I understand that to emulate Gremlin,
>> >>>>>> the traversal methods etc. should use CamelCase. But I wonder if
>> >>>>>> the
>> >>>>>> helper classes (Translators) should use the Python convention of
>> >>>>>> using
>> >>>>>> underscores to name methods. Python class names use camel case by
>> >>>>>> convention.
>> >>>>>
>> >>>>>
>> >>>>> I haven’t changed it. If you feel we should, please do. And yes, I
>> >>>>> think
>> >>>>> its good to keep the Gremlin step names camelCase. For Gremlin-Ruby,
>> >>>>> we
>> >>>>> will do out_e() style.
>> >>>>>
>> >>>>>> Finally, the implementation of the B class may need some work, but
>> >>>>>> we'll have to play around with it a bit to figure out how the best
>> >>>>>> approach to doing this.
>> >>>>>
>> >>>>>
>> >>>>> Yea, thats all wrong. I overdosed on the introspection and then
>> >>>>> Kuppitz
>> >>>>> was like “Why not just have it as….” (something much simpler — which
>> >>>>> I
>> >>>>> forget what it was now).
>> >>>>>
>> >>>>>> I'm sure there are more improvements, but I just wanted to get a
>> >>>>>> conversation going. I would be happy to make a PR with some of
>> >>>>>> these
>> >>>>>> changes.
>> >>>>>
>> >>>>> That’d be awesome. Its in TINKERPOP-1278 branch. In
>> >>>>> gremlin-variant/test
>> >>>>> you will see PythonProcessStandardTest and
>> >>>>> PythonProcessComputerTest. Those
>> >>>>> verify that the compilation is valid for all the standard and
>> >>>>> computer
>> >>>>> tests.
>> >>>>>
>> >>>>>> Also, gremlinclient will soon support the RemoteConnection
>> >>>>>> interface,
>> >>>>>> I'll send out a link to the docs once I get everything up and
>> >>>>>> running.
>> >>>>>
>> >>>>> So cool. Please do review the Python RemoteConnection class as
>> >>>>> again, I
>> >>>>> just improv’d it.
>> >>>>>
>> >>>>> Thanks again David,
>> >>>>> Marko.
>> >>>>>
>> >>>>> http://markorodriguez.com
>> >>>
>> >>>
>> >>>
>> >>> --
>> >>> David M. Brown
>> >>> R.A. CulturePlex Lab, Western University
>> >>
>> >
>> >
>> >
>> > --
>> > David M. Brown
>> > R.A. CulturePlex Lab, Western University
>>
>
--
David M. Brown
R.A. CulturePlex Lab, Western University
Re: gremlin_python GLV
Posted by David Brown <da...@gmail.com>.
Are u using websocket or http?
On Jun 15, 2016 17:09, "Marko Rodriguez" <ok...@gmail.com> wrote:
> Dar.
>
> >>> g.V().name.toList()
> Traceback (most recent call last):
> File "<stdin>", line 1, in <module>
> File
> "/Users/marko/software/tinkerpop/gremlin-variant/src/main/jython/gremlin_python/gremlin_python.py",
> line 107, in toList
> return list(iter(self))
> File
> "/Users/marko/software/tinkerpop/gremlin-variant/src/main/jython/gremlin_python/gremlin_python.py",
> line 110, in next
> self.results =
> self.remote_connection.submit(self.translator.script_engine,
> self.translator.traversal_script, self.bindings)
> File
> "/Library/Python/2.7/site-packages/gremlinclient/tornado_client/remote_connection.py",
> line 20, in submit
> results = self._loop.run_sync(lambda:
> File "/Library/Python/2.7/site-packages/tornado/ioloop.py", line 453, in
> run_sync
> return future_cell[0].result()
> File "/Library/Python/2.7/site-packages/tornado/concurrent.py", line
> 232, in result
> raise_exc_info(self._exc_info)
> File "/Library/Python/2.7/site-packages/tornado/gen.py", line 1014, in
> run
> yielded = self.gen.throw(*exc_info)
> File
> "/Library/Python/2.7/site-packages/gremlinclient/tornado_client/remote_connection.py",
> line 27, in _execute
> conn = yield self._pool.acquire()
> File "/Library/Python/2.7/site-packages/tornado/gen.py", line 1008, in
> run
> value = future.result()
> File "/Library/Python/2.7/site-packages/tornado/concurrent.py", line
> 232, in result
> raise_exc_info(self._exc_info)
> File "<string>", line 3, in raise_exc_info
> tornado.httpclient.HTTPError: HTTP 400: Bad Request
>
>
>
>
> > On Jun 15, 2016, at 5:36 PM, David Brown <da...@gmail.com> wrote:
> >
> > Nice Leif!
> >
> > I just added very basic support for the RemoteConnection interface in
> > gremlinclient:
> >
> >
> http://gremlinclient.readthedocs.io/en/latest/usage.html#the-remoteconnection-object
> >
> > It needs more tests etc., but it seems to be working well. If you want
> > to try it out, you can pip install gremlinclient from github:
> >
> > pip install git+https://github.com/davebshow/gremlinclient.git
> >
> > More tomorrow...
> >
> > On Wed, Jun 15, 2016 at 7:17 PM, Leifur Halldor Asgeirsson
> > <LA...@zerofail.com> wrote:
> >> Thanks! I'll submit my PR in the morning.
> >>
> >> ________________________________________
> >> From: Marko Rodriguez <ok...@gmail.com>
> >> Sent: June 15, 2016 6:13 PM
> >> To: dev@tinkerpop.apache.org
> >> Subject: Re: gremlin_python GLV
> >>
> >> Hi,
> >>
> >> That is a really good idea.
> >>
> >> Want to do a PR to the branch or do you want me to just take your
> notes/gist and make it happen?
> >>
> >> Marko.
> >>
> >> http://markorodriguez.com
> >>
> >>
> >>
> >>> On Jun 15, 2016, at 3:55 PM, Leifur Halldor Asgeirsson <
> LAsgeirsson@zerofail.com> wrote:
> >>>
> >>> Hi all,
> >>> I have a couple of suggestions for the GLV.
> >>>
> >>> Firstly, it would be useful to be able to inject arbitrary
> expressions, such as static constructors. For example, in an application
> that I'm currently working on, I use Titan's Geoshape property type. I
> would like to be able to call one of the static constructors on the
> Geoshape class in a script, passing it bound parameters. Something like
> >>>
> >>> "g.V().has('location', geoWithin(Geoshape.circle(lat, lon, radius)))"
> >>> with bound parameters {'lat': 45, 'lon': 45, 'radius': '10'}
> >>>
> >>> I took an initial stab at this today, and this is what I came up with:
> >>> https://gist.github.com/leifurhauks/5a843379183123dbed1134a92691a335
> >>>
> >>> With those changes, I can use that static constructor in a traversal
> as follows:
> >>>
> >>>>>> translator = GroovyTranslator('g')
> >>>>>> g = PythonGraphTraversalSource(translator)
> >>>>>> t = g.V().has('location', RawExpression('Geoshape.point(', B('lat',
> 45), ', ', B('lon', 45), ')'))
> >>>>>> str(t)
> >>> 'g.V().has("location", Geoshape.point(lat, lon))'
> >>>>>> t.bindings
> >>> {'lon': 45, 'lat': 45}
> >>>
> >>> That raw expression isn't very readable, but a simple helper class can
> fix that:
> >>>
> >>> class Geoshape(object):
> >>> @staticmethod
> >>> def point(latitude, longitude, symbols=('lat', 'lon')):
> >>> return RawExpression(
> >>> 'Geoshape.point(', B(symbols[0], latitude), ', ',
> B(symbols[1], longitude), ')')
> >>>
> >>> Now I can rewrite the previous traversal to be much clearer:
> >>>
> >>> t = g.V().has('location', Geoshape.point(45, 45))
> >>>
> >>>
> >>> If this seems like a reasonable approach, I would be happy to submit a
> PR.
> >>>
> >>>
> >>> I have one other suggestion, but this one is tiny. Because most of the
> steps on PythonGraphTraversal have the same implementation, it would be
> possible to specify that implementation once in a function factory, like
> this:
> >>>
> >>> def simple_step(name):
> >>> def step_method(self, *args):
> >>> self.translator.addStep(self, name, *args)
> >>> for arg in args:
> >>> if type(arg) is B:
> >>> self.bindings[arg.symbol] = arg.value
> >>> return self
> >>> return step_method
> >>>
> >>> Then, on PythonGraphTraversal, all the step methods that use that
> implementation could be declared like this:
> >>>
> >>> class PythonGraphTraversal(object):
> >>> def __init__(self, translator, remote_connection=None):
> >>> # elided ...
> >>> # top methods elided
> >>> has = simple_step('has')
> >>> hasId = simple_step('hasId')
> >>> hasKey = smple_step('hasKey')
> >>> # and so on...
> >>>
> >>>
> >>> ________________________________________
> >>> From: David Brown <da...@gmail.com>
> >>> Sent: June 15, 2016 3:23 PM
> >>> To: dev@tinkerpop.apache.org
> >>> Subject: Re: gremlin_python GLV
> >>>
> >>> Ok Stephen I will. I can't claim to be an expert (especially when it
> >>> comes to the Java ecosystem), but I will definitely take a look.
> >>>
> >>> Also, Marko, Python doesn't really have primitives as such. The built
> >>> in function isinstance should work for everything in gremlin_python
> >>> e.g. ``isinstance(var, bool)``. I can take a look at this when I make
> >>> a PR if you'd like, but it really isn't a huge deal anyway.
> >>>
> >>> On Wed, Jun 15, 2016 at 3:16 PM, Stephen Mallette <
> spmallette@gmail.com> wrote:
> >>>> David, it would also be great to get your feedback on the
> >>>> packaging/deployment approach we have so far. I have some basic
> figured out
> >>>> for deployment to pypi over maven via twine, but it could use an
> experts
> >>>> eye. You probably don't need to check that part out now as you are
> already
> >>>> have some other stuff to look at, but I just wanted to mention that
> so that
> >>>> it was in your mind for later. Thanks for your help on this.
> >>>>
> >>>> On Wed, Jun 15, 2016 at 3:02 PM, Marko Rodriguez <
> okrammarko@gmail.com>
> >>>> wrote:
> >>>>
> >>>>> Hello,
> >>>>>
> >>>>>> I was reading through the gremlin_python GLV code this morning, and
> >>>>>> overall it looks like it should work pretty smoothly. Thanks for
> doing
> >>>>>> this Marko, really cool work! I just wanted to comment on a few
> things
> >>>>>> that popped out at me on my first reading.The major issue I see is
> >>>>>> that it is not currently Python 2/3 compatible. This is due to two
> >>>>>> things:
> >>>>>
> >>>>> Cool! Thank you for taking your time to review the work.
> >>>>>
> >>>>>> 1. The way iterators are implemented in Python 2/3 is different.
> This
> >>>>>> problem could be remedied by adding a method to
> >>>>>> ``PythonGraphTraversal``, something like:
> >>>>>>
> >>>>>> def __next__(self):
> >>>>>> return self.next()
> >>>>>>
> >>>>>> then, in the ``next`` method, changing line 113 to
> >>>>>> ``next(self.results)``, should take care of this problem.
> >>>>>
> >>>>> Updated.
> >>>>>
> >>>>>> 2. The ``GroovyTranslator`` class checks for type ``long``, this no
> >>>>>> longer exists in Python 3. Determining what to submit as a Long
> might
> >>>>>> require some discussion, but this could be easily fixed by adding
> >>>>>> something like:
> >>>>>>
> >>>>>> import sys
> >>>>>> if sys.version_info.major > 2:
> >>>>>> long = int
> >>>>>>
> >>>>>> to the top of the file.
> >>>>>
> >>>>> Updated.
> >>>>>
> >>>>>>
> >>>>>> Other than this, there are some minor details that could be cleaned
> >>>>>> up. Particularly:
> >>>>>>
> >>>>>> 1. Using ``isinstance`` instead of ``type`` to perform type
> checking.
> >>>>>> This will recognize subclasses and is the most Pythonic way to do
> >>>>>> this.
> >>>>>
> >>>>> Seems isinstance() isn’t a method on primitives — only objects.
> Thus, the
> >>>>> code got complex. Left it with type().
> >>>>>
> >>>>>>
> >>>>>> 2. Formatting - indents, and line spacing. Typically, Python methods
> >>>>>> are separated by a single line, and indents use four spaces. This is
> >>>>>> really just cosmetic for readability.
> >>>>>
> >>>>> Uh. The problem is that the source code is auto-generated from
> >>>>> GremlinPythonGenerator. If you want to tweak, please do so:
> >>>>>
> >>>>>
> >>>>>
> https://github.com/apache/tinkerpop/blob/TINKERPOP-1278/gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy
> >>>>> <
> >>>>>
> https://github.com/apache/tinkerpop/blob/TINKERPOP-1278/gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy
> >>>>>>
> >>>>>
> >>>>>
> >>>>>> 3. CamelCase vs. underscores. I understand that to emulate Gremlin,
> >>>>>> the traversal methods etc. should use CamelCase. But I wonder if the
> >>>>>> helper classes (Translators) should use the Python convention of
> using
> >>>>>> underscores to name methods. Python class names use camel case by
> >>>>>> convention.
> >>>>>
> >>>>>
> >>>>> I haven’t changed it. If you feel we should, please do. And yes, I
> think
> >>>>> its good to keep the Gremlin step names camelCase. For Gremlin-Ruby,
> we
> >>>>> will do out_e() style.
> >>>>>
> >>>>>> Finally, the implementation of the B class may need some work, but
> >>>>>> we'll have to play around with it a bit to figure out how the best
> >>>>>> approach to doing this.
> >>>>>
> >>>>>
> >>>>> Yea, thats all wrong. I overdosed on the introspection and then
> Kuppitz
> >>>>> was like “Why not just have it as….” (something much simpler — which
> I
> >>>>> forget what it was now).
> >>>>>
> >>>>>> I'm sure there are more improvements, but I just wanted to get a
> >>>>>> conversation going. I would be happy to make a PR with some of these
> >>>>>> changes.
> >>>>>
> >>>>> That’d be awesome. Its in TINKERPOP-1278 branch. In
> gremlin-variant/test
> >>>>> you will see PythonProcessStandardTest and
> PythonProcessComputerTest. Those
> >>>>> verify that the compilation is valid for all the standard and
> computer
> >>>>> tests.
> >>>>>
> >>>>>> Also, gremlinclient will soon support the RemoteConnection
> interface,
> >>>>>> I'll send out a link to the docs once I get everything up and
> running.
> >>>>>
> >>>>> So cool. Please do review the Python RemoteConnection class as
> again, I
> >>>>> just improv’d it.
> >>>>>
> >>>>> Thanks again David,
> >>>>> Marko.
> >>>>>
> >>>>> http://markorodriguez.com
> >>>
> >>>
> >>>
> >>> --
> >>> David M. Brown
> >>> R.A. CulturePlex Lab, Western University
> >>
> >
> >
> >
> > --
> > David M. Brown
> > R.A. CulturePlex Lab, Western University
>
>
Re: gremlin_python GLV
Posted by Marko Rodriguez <ok...@gmail.com>.
Dar.
>>> g.V().name.toList()
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/marko/software/tinkerpop/gremlin-variant/src/main/jython/gremlin_python/gremlin_python.py", line 107, in toList
return list(iter(self))
File "/Users/marko/software/tinkerpop/gremlin-variant/src/main/jython/gremlin_python/gremlin_python.py", line 110, in next
self.results = self.remote_connection.submit(self.translator.script_engine, self.translator.traversal_script, self.bindings)
File "/Library/Python/2.7/site-packages/gremlinclient/tornado_client/remote_connection.py", line 20, in submit
results = self._loop.run_sync(lambda:
File "/Library/Python/2.7/site-packages/tornado/ioloop.py", line 453, in run_sync
return future_cell[0].result()
File "/Library/Python/2.7/site-packages/tornado/concurrent.py", line 232, in result
raise_exc_info(self._exc_info)
File "/Library/Python/2.7/site-packages/tornado/gen.py", line 1014, in run
yielded = self.gen.throw(*exc_info)
File "/Library/Python/2.7/site-packages/gremlinclient/tornado_client/remote_connection.py", line 27, in _execute
conn = yield self._pool.acquire()
File "/Library/Python/2.7/site-packages/tornado/gen.py", line 1008, in run
value = future.result()
File "/Library/Python/2.7/site-packages/tornado/concurrent.py", line 232, in result
raise_exc_info(self._exc_info)
File "<string>", line 3, in raise_exc_info
tornado.httpclient.HTTPError: HTTP 400: Bad Request
> On Jun 15, 2016, at 5:36 PM, David Brown <da...@gmail.com> wrote:
>
> Nice Leif!
>
> I just added very basic support for the RemoteConnection interface in
> gremlinclient:
>
> http://gremlinclient.readthedocs.io/en/latest/usage.html#the-remoteconnection-object
>
> It needs more tests etc., but it seems to be working well. If you want
> to try it out, you can pip install gremlinclient from github:
>
> pip install git+https://github.com/davebshow/gremlinclient.git
>
> More tomorrow...
>
> On Wed, Jun 15, 2016 at 7:17 PM, Leifur Halldor Asgeirsson
> <LA...@zerofail.com> wrote:
>> Thanks! I'll submit my PR in the morning.
>>
>> ________________________________________
>> From: Marko Rodriguez <ok...@gmail.com>
>> Sent: June 15, 2016 6:13 PM
>> To: dev@tinkerpop.apache.org
>> Subject: Re: gremlin_python GLV
>>
>> Hi,
>>
>> That is a really good idea.
>>
>> Want to do a PR to the branch or do you want me to just take your notes/gist and make it happen?
>>
>> Marko.
>>
>> http://markorodriguez.com
>>
>>
>>
>>> On Jun 15, 2016, at 3:55 PM, Leifur Halldor Asgeirsson <LA...@zerofail.com> wrote:
>>>
>>> Hi all,
>>> I have a couple of suggestions for the GLV.
>>>
>>> Firstly, it would be useful to be able to inject arbitrary expressions, such as static constructors. For example, in an application that I'm currently working on, I use Titan's Geoshape property type. I would like to be able to call one of the static constructors on the Geoshape class in a script, passing it bound parameters. Something like
>>>
>>> "g.V().has('location', geoWithin(Geoshape.circle(lat, lon, radius)))"
>>> with bound parameters {'lat': 45, 'lon': 45, 'radius': '10'}
>>>
>>> I took an initial stab at this today, and this is what I came up with:
>>> https://gist.github.com/leifurhauks/5a843379183123dbed1134a92691a335
>>>
>>> With those changes, I can use that static constructor in a traversal as follows:
>>>
>>>>>> translator = GroovyTranslator('g')
>>>>>> g = PythonGraphTraversalSource(translator)
>>>>>> t = g.V().has('location', RawExpression('Geoshape.point(', B('lat', 45), ', ', B('lon', 45), ')'))
>>>>>> str(t)
>>> 'g.V().has("location", Geoshape.point(lat, lon))'
>>>>>> t.bindings
>>> {'lon': 45, 'lat': 45}
>>>
>>> That raw expression isn't very readable, but a simple helper class can fix that:
>>>
>>> class Geoshape(object):
>>> @staticmethod
>>> def point(latitude, longitude, symbols=('lat', 'lon')):
>>> return RawExpression(
>>> 'Geoshape.point(', B(symbols[0], latitude), ', ', B(symbols[1], longitude), ')')
>>>
>>> Now I can rewrite the previous traversal to be much clearer:
>>>
>>> t = g.V().has('location', Geoshape.point(45, 45))
>>>
>>>
>>> If this seems like a reasonable approach, I would be happy to submit a PR.
>>>
>>>
>>> I have one other suggestion, but this one is tiny. Because most of the steps on PythonGraphTraversal have the same implementation, it would be possible to specify that implementation once in a function factory, like this:
>>>
>>> def simple_step(name):
>>> def step_method(self, *args):
>>> self.translator.addStep(self, name, *args)
>>> for arg in args:
>>> if type(arg) is B:
>>> self.bindings[arg.symbol] = arg.value
>>> return self
>>> return step_method
>>>
>>> Then, on PythonGraphTraversal, all the step methods that use that implementation could be declared like this:
>>>
>>> class PythonGraphTraversal(object):
>>> def __init__(self, translator, remote_connection=None):
>>> # elided ...
>>> # top methods elided
>>> has = simple_step('has')
>>> hasId = simple_step('hasId')
>>> hasKey = smple_step('hasKey')
>>> # and so on...
>>>
>>>
>>> ________________________________________
>>> From: David Brown <da...@gmail.com>
>>> Sent: June 15, 2016 3:23 PM
>>> To: dev@tinkerpop.apache.org
>>> Subject: Re: gremlin_python GLV
>>>
>>> Ok Stephen I will. I can't claim to be an expert (especially when it
>>> comes to the Java ecosystem), but I will definitely take a look.
>>>
>>> Also, Marko, Python doesn't really have primitives as such. The built
>>> in function isinstance should work for everything in gremlin_python
>>> e.g. ``isinstance(var, bool)``. I can take a look at this when I make
>>> a PR if you'd like, but it really isn't a huge deal anyway.
>>>
>>> On Wed, Jun 15, 2016 at 3:16 PM, Stephen Mallette <sp...@gmail.com> wrote:
>>>> David, it would also be great to get your feedback on the
>>>> packaging/deployment approach we have so far. I have some basic figured out
>>>> for deployment to pypi over maven via twine, but it could use an experts
>>>> eye. You probably don't need to check that part out now as you are already
>>>> have some other stuff to look at, but I just wanted to mention that so that
>>>> it was in your mind for later. Thanks for your help on this.
>>>>
>>>> On Wed, Jun 15, 2016 at 3:02 PM, Marko Rodriguez <ok...@gmail.com>
>>>> wrote:
>>>>
>>>>> Hello,
>>>>>
>>>>>> I was reading through the gremlin_python GLV code this morning, and
>>>>>> overall it looks like it should work pretty smoothly. Thanks for doing
>>>>>> this Marko, really cool work! I just wanted to comment on a few things
>>>>>> that popped out at me on my first reading.The major issue I see is
>>>>>> that it is not currently Python 2/3 compatible. This is due to two
>>>>>> things:
>>>>>
>>>>> Cool! Thank you for taking your time to review the work.
>>>>>
>>>>>> 1. The way iterators are implemented in Python 2/3 is different. This
>>>>>> problem could be remedied by adding a method to
>>>>>> ``PythonGraphTraversal``, something like:
>>>>>>
>>>>>> def __next__(self):
>>>>>> return self.next()
>>>>>>
>>>>>> then, in the ``next`` method, changing line 113 to
>>>>>> ``next(self.results)``, should take care of this problem.
>>>>>
>>>>> Updated.
>>>>>
>>>>>> 2. The ``GroovyTranslator`` class checks for type ``long``, this no
>>>>>> longer exists in Python 3. Determining what to submit as a Long might
>>>>>> require some discussion, but this could be easily fixed by adding
>>>>>> something like:
>>>>>>
>>>>>> import sys
>>>>>> if sys.version_info.major > 2:
>>>>>> long = int
>>>>>>
>>>>>> to the top of the file.
>>>>>
>>>>> Updated.
>>>>>
>>>>>>
>>>>>> Other than this, there are some minor details that could be cleaned
>>>>>> up. Particularly:
>>>>>>
>>>>>> 1. Using ``isinstance`` instead of ``type`` to perform type checking.
>>>>>> This will recognize subclasses and is the most Pythonic way to do
>>>>>> this.
>>>>>
>>>>> Seems isinstance() isn’t a method on primitives — only objects. Thus, the
>>>>> code got complex. Left it with type().
>>>>>
>>>>>>
>>>>>> 2. Formatting - indents, and line spacing. Typically, Python methods
>>>>>> are separated by a single line, and indents use four spaces. This is
>>>>>> really just cosmetic for readability.
>>>>>
>>>>> Uh. The problem is that the source code is auto-generated from
>>>>> GremlinPythonGenerator. If you want to tweak, please do so:
>>>>>
>>>>>
>>>>> https://github.com/apache/tinkerpop/blob/TINKERPOP-1278/gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy
>>>>> <
>>>>> https://github.com/apache/tinkerpop/blob/TINKERPOP-1278/gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy
>>>>>>
>>>>>
>>>>>
>>>>>> 3. CamelCase vs. underscores. I understand that to emulate Gremlin,
>>>>>> the traversal methods etc. should use CamelCase. But I wonder if the
>>>>>> helper classes (Translators) should use the Python convention of using
>>>>>> underscores to name methods. Python class names use camel case by
>>>>>> convention.
>>>>>
>>>>>
>>>>> I haven’t changed it. If you feel we should, please do. And yes, I think
>>>>> its good to keep the Gremlin step names camelCase. For Gremlin-Ruby, we
>>>>> will do out_e() style.
>>>>>
>>>>>> Finally, the implementation of the B class may need some work, but
>>>>>> we'll have to play around with it a bit to figure out how the best
>>>>>> approach to doing this.
>>>>>
>>>>>
>>>>> Yea, thats all wrong. I overdosed on the introspection and then Kuppitz
>>>>> was like “Why not just have it as….” (something much simpler — which I
>>>>> forget what it was now).
>>>>>
>>>>>> I'm sure there are more improvements, but I just wanted to get a
>>>>>> conversation going. I would be happy to make a PR with some of these
>>>>>> changes.
>>>>>
>>>>> That’d be awesome. Its in TINKERPOP-1278 branch. In gremlin-variant/test
>>>>> you will see PythonProcessStandardTest and PythonProcessComputerTest. Those
>>>>> verify that the compilation is valid for all the standard and computer
>>>>> tests.
>>>>>
>>>>>> Also, gremlinclient will soon support the RemoteConnection interface,
>>>>>> I'll send out a link to the docs once I get everything up and running.
>>>>>
>>>>> So cool. Please do review the Python RemoteConnection class as again, I
>>>>> just improv’d it.
>>>>>
>>>>> Thanks again David,
>>>>> Marko.
>>>>>
>>>>> http://markorodriguez.com
>>>
>>>
>>>
>>> --
>>> David M. Brown
>>> R.A. CulturePlex Lab, Western University
>>
>
>
>
> --
> David M. Brown
> R.A. CulturePlex Lab, Western University
Re: gremlin_python GLV
Posted by David Brown <da...@gmail.com>.
Nice Leif!
I just added very basic support for the RemoteConnection interface in
gremlinclient:
http://gremlinclient.readthedocs.io/en/latest/usage.html#the-remoteconnection-object
It needs more tests etc., but it seems to be working well. If you want
to try it out, you can pip install gremlinclient from github:
pip install git+https://github.com/davebshow/gremlinclient.git
More tomorrow...
On Wed, Jun 15, 2016 at 7:17 PM, Leifur Halldor Asgeirsson
<LA...@zerofail.com> wrote:
> Thanks! I'll submit my PR in the morning.
>
> ________________________________________
> From: Marko Rodriguez <ok...@gmail.com>
> Sent: June 15, 2016 6:13 PM
> To: dev@tinkerpop.apache.org
> Subject: Re: gremlin_python GLV
>
> Hi,
>
> That is a really good idea.
>
> Want to do a PR to the branch or do you want me to just take your notes/gist and make it happen?
>
> Marko.
>
> http://markorodriguez.com
>
>
>
>> On Jun 15, 2016, at 3:55 PM, Leifur Halldor Asgeirsson <LA...@zerofail.com> wrote:
>>
>> Hi all,
>> I have a couple of suggestions for the GLV.
>>
>> Firstly, it would be useful to be able to inject arbitrary expressions, such as static constructors. For example, in an application that I'm currently working on, I use Titan's Geoshape property type. I would like to be able to call one of the static constructors on the Geoshape class in a script, passing it bound parameters. Something like
>>
>> "g.V().has('location', geoWithin(Geoshape.circle(lat, lon, radius)))"
>> with bound parameters {'lat': 45, 'lon': 45, 'radius': '10'}
>>
>> I took an initial stab at this today, and this is what I came up with:
>> https://gist.github.com/leifurhauks/5a843379183123dbed1134a92691a335
>>
>> With those changes, I can use that static constructor in a traversal as follows:
>>
>>>>> translator = GroovyTranslator('g')
>>>>> g = PythonGraphTraversalSource(translator)
>>>>> t = g.V().has('location', RawExpression('Geoshape.point(', B('lat', 45), ', ', B('lon', 45), ')'))
>>>>> str(t)
>> 'g.V().has("location", Geoshape.point(lat, lon))'
>>>>> t.bindings
>> {'lon': 45, 'lat': 45}
>>
>> That raw expression isn't very readable, but a simple helper class can fix that:
>>
>> class Geoshape(object):
>> @staticmethod
>> def point(latitude, longitude, symbols=('lat', 'lon')):
>> return RawExpression(
>> 'Geoshape.point(', B(symbols[0], latitude), ', ', B(symbols[1], longitude), ')')
>>
>> Now I can rewrite the previous traversal to be much clearer:
>>
>> t = g.V().has('location', Geoshape.point(45, 45))
>>
>>
>> If this seems like a reasonable approach, I would be happy to submit a PR.
>>
>>
>> I have one other suggestion, but this one is tiny. Because most of the steps on PythonGraphTraversal have the same implementation, it would be possible to specify that implementation once in a function factory, like this:
>>
>> def simple_step(name):
>> def step_method(self, *args):
>> self.translator.addStep(self, name, *args)
>> for arg in args:
>> if type(arg) is B:
>> self.bindings[arg.symbol] = arg.value
>> return self
>> return step_method
>>
>> Then, on PythonGraphTraversal, all the step methods that use that implementation could be declared like this:
>>
>> class PythonGraphTraversal(object):
>> def __init__(self, translator, remote_connection=None):
>> # elided ...
>> # top methods elided
>> has = simple_step('has')
>> hasId = simple_step('hasId')
>> hasKey = smple_step('hasKey')
>> # and so on...
>>
>>
>> ________________________________________
>> From: David Brown <da...@gmail.com>
>> Sent: June 15, 2016 3:23 PM
>> To: dev@tinkerpop.apache.org
>> Subject: Re: gremlin_python GLV
>>
>> Ok Stephen I will. I can't claim to be an expert (especially when it
>> comes to the Java ecosystem), but I will definitely take a look.
>>
>> Also, Marko, Python doesn't really have primitives as such. The built
>> in function isinstance should work for everything in gremlin_python
>> e.g. ``isinstance(var, bool)``. I can take a look at this when I make
>> a PR if you'd like, but it really isn't a huge deal anyway.
>>
>> On Wed, Jun 15, 2016 at 3:16 PM, Stephen Mallette <sp...@gmail.com> wrote:
>>> David, it would also be great to get your feedback on the
>>> packaging/deployment approach we have so far. I have some basic figured out
>>> for deployment to pypi over maven via twine, but it could use an experts
>>> eye. You probably don't need to check that part out now as you are already
>>> have some other stuff to look at, but I just wanted to mention that so that
>>> it was in your mind for later. Thanks for your help on this.
>>>
>>> On Wed, Jun 15, 2016 at 3:02 PM, Marko Rodriguez <ok...@gmail.com>
>>> wrote:
>>>
>>>> Hello,
>>>>
>>>>> I was reading through the gremlin_python GLV code this morning, and
>>>>> overall it looks like it should work pretty smoothly. Thanks for doing
>>>>> this Marko, really cool work! I just wanted to comment on a few things
>>>>> that popped out at me on my first reading.The major issue I see is
>>>>> that it is not currently Python 2/3 compatible. This is due to two
>>>>> things:
>>>>
>>>> Cool! Thank you for taking your time to review the work.
>>>>
>>>>> 1. The way iterators are implemented in Python 2/3 is different. This
>>>>> problem could be remedied by adding a method to
>>>>> ``PythonGraphTraversal``, something like:
>>>>>
>>>>> def __next__(self):
>>>>> return self.next()
>>>>>
>>>>> then, in the ``next`` method, changing line 113 to
>>>>> ``next(self.results)``, should take care of this problem.
>>>>
>>>> Updated.
>>>>
>>>>> 2. The ``GroovyTranslator`` class checks for type ``long``, this no
>>>>> longer exists in Python 3. Determining what to submit as a Long might
>>>>> require some discussion, but this could be easily fixed by adding
>>>>> something like:
>>>>>
>>>>> import sys
>>>>> if sys.version_info.major > 2:
>>>>> long = int
>>>>>
>>>>> to the top of the file.
>>>>
>>>> Updated.
>>>>
>>>>>
>>>>> Other than this, there are some minor details that could be cleaned
>>>>> up. Particularly:
>>>>>
>>>>> 1. Using ``isinstance`` instead of ``type`` to perform type checking.
>>>>> This will recognize subclasses and is the most Pythonic way to do
>>>>> this.
>>>>
>>>> Seems isinstance() isn’t a method on primitives — only objects. Thus, the
>>>> code got complex. Left it with type().
>>>>
>>>>>
>>>>> 2. Formatting - indents, and line spacing. Typically, Python methods
>>>>> are separated by a single line, and indents use four spaces. This is
>>>>> really just cosmetic for readability.
>>>>
>>>> Uh. The problem is that the source code is auto-generated from
>>>> GremlinPythonGenerator. If you want to tweak, please do so:
>>>>
>>>>
>>>> https://github.com/apache/tinkerpop/blob/TINKERPOP-1278/gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy
>>>> <
>>>> https://github.com/apache/tinkerpop/blob/TINKERPOP-1278/gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy
>>>>>
>>>>
>>>>
>>>>> 3. CamelCase vs. underscores. I understand that to emulate Gremlin,
>>>>> the traversal methods etc. should use CamelCase. But I wonder if the
>>>>> helper classes (Translators) should use the Python convention of using
>>>>> underscores to name methods. Python class names use camel case by
>>>>> convention.
>>>>
>>>>
>>>> I haven’t changed it. If you feel we should, please do. And yes, I think
>>>> its good to keep the Gremlin step names camelCase. For Gremlin-Ruby, we
>>>> will do out_e() style.
>>>>
>>>>> Finally, the implementation of the B class may need some work, but
>>>>> we'll have to play around with it a bit to figure out how the best
>>>>> approach to doing this.
>>>>
>>>>
>>>> Yea, thats all wrong. I overdosed on the introspection and then Kuppitz
>>>> was like “Why not just have it as….” (something much simpler — which I
>>>> forget what it was now).
>>>>
>>>>> I'm sure there are more improvements, but I just wanted to get a
>>>>> conversation going. I would be happy to make a PR with some of these
>>>>> changes.
>>>>
>>>> That’d be awesome. Its in TINKERPOP-1278 branch. In gremlin-variant/test
>>>> you will see PythonProcessStandardTest and PythonProcessComputerTest. Those
>>>> verify that the compilation is valid for all the standard and computer
>>>> tests.
>>>>
>>>>> Also, gremlinclient will soon support the RemoteConnection interface,
>>>>> I'll send out a link to the docs once I get everything up and running.
>>>>
>>>> So cool. Please do review the Python RemoteConnection class as again, I
>>>> just improv’d it.
>>>>
>>>> Thanks again David,
>>>> Marko.
>>>>
>>>> http://markorodriguez.com
>>
>>
>>
>> --
>> David M. Brown
>> R.A. CulturePlex Lab, Western University
>
--
David M. Brown
R.A. CulturePlex Lab, Western University
Re: gremlin_python GLV
Posted by Leifur Halldor Asgeirsson <LA...@zerofail.com>.
Thanks! I'll submit my PR in the morning.
________________________________________
From: Marko Rodriguez <ok...@gmail.com>
Sent: June 15, 2016 6:13 PM
To: dev@tinkerpop.apache.org
Subject: Re: gremlin_python GLV
Hi,
That is a really good idea.
Want to do a PR to the branch or do you want me to just take your notes/gist and make it happen?
Marko.
http://markorodriguez.com
> On Jun 15, 2016, at 3:55 PM, Leifur Halldor Asgeirsson <LA...@zerofail.com> wrote:
>
> Hi all,
> I have a couple of suggestions for the GLV.
>
> Firstly, it would be useful to be able to inject arbitrary expressions, such as static constructors. For example, in an application that I'm currently working on, I use Titan's Geoshape property type. I would like to be able to call one of the static constructors on the Geoshape class in a script, passing it bound parameters. Something like
>
> "g.V().has('location', geoWithin(Geoshape.circle(lat, lon, radius)))"
> with bound parameters {'lat': 45, 'lon': 45, 'radius': '10'}
>
> I took an initial stab at this today, and this is what I came up with:
> https://gist.github.com/leifurhauks/5a843379183123dbed1134a92691a335
>
> With those changes, I can use that static constructor in a traversal as follows:
>
>>>> translator = GroovyTranslator('g')
>>>> g = PythonGraphTraversalSource(translator)
>>>> t = g.V().has('location', RawExpression('Geoshape.point(', B('lat', 45), ', ', B('lon', 45), ')'))
>>>> str(t)
> 'g.V().has("location", Geoshape.point(lat, lon))'
>>>> t.bindings
> {'lon': 45, 'lat': 45}
>
> That raw expression isn't very readable, but a simple helper class can fix that:
>
> class Geoshape(object):
> @staticmethod
> def point(latitude, longitude, symbols=('lat', 'lon')):
> return RawExpression(
> 'Geoshape.point(', B(symbols[0], latitude), ', ', B(symbols[1], longitude), ')')
>
> Now I can rewrite the previous traversal to be much clearer:
>
> t = g.V().has('location', Geoshape.point(45, 45))
>
>
> If this seems like a reasonable approach, I would be happy to submit a PR.
>
>
> I have one other suggestion, but this one is tiny. Because most of the steps on PythonGraphTraversal have the same implementation, it would be possible to specify that implementation once in a function factory, like this:
>
> def simple_step(name):
> def step_method(self, *args):
> self.translator.addStep(self, name, *args)
> for arg in args:
> if type(arg) is B:
> self.bindings[arg.symbol] = arg.value
> return self
> return step_method
>
> Then, on PythonGraphTraversal, all the step methods that use that implementation could be declared like this:
>
> class PythonGraphTraversal(object):
> def __init__(self, translator, remote_connection=None):
> # elided ...
> # top methods elided
> has = simple_step('has')
> hasId = simple_step('hasId')
> hasKey = smple_step('hasKey')
> # and so on...
>
>
> ________________________________________
> From: David Brown <da...@gmail.com>
> Sent: June 15, 2016 3:23 PM
> To: dev@tinkerpop.apache.org
> Subject: Re: gremlin_python GLV
>
> Ok Stephen I will. I can't claim to be an expert (especially when it
> comes to the Java ecosystem), but I will definitely take a look.
>
> Also, Marko, Python doesn't really have primitives as such. The built
> in function isinstance should work for everything in gremlin_python
> e.g. ``isinstance(var, bool)``. I can take a look at this when I make
> a PR if you'd like, but it really isn't a huge deal anyway.
>
> On Wed, Jun 15, 2016 at 3:16 PM, Stephen Mallette <sp...@gmail.com> wrote:
>> David, it would also be great to get your feedback on the
>> packaging/deployment approach we have so far. I have some basic figured out
>> for deployment to pypi over maven via twine, but it could use an experts
>> eye. You probably don't need to check that part out now as you are already
>> have some other stuff to look at, but I just wanted to mention that so that
>> it was in your mind for later. Thanks for your help on this.
>>
>> On Wed, Jun 15, 2016 at 3:02 PM, Marko Rodriguez <ok...@gmail.com>
>> wrote:
>>
>>> Hello,
>>>
>>>> I was reading through the gremlin_python GLV code this morning, and
>>>> overall it looks like it should work pretty smoothly. Thanks for doing
>>>> this Marko, really cool work! I just wanted to comment on a few things
>>>> that popped out at me on my first reading.The major issue I see is
>>>> that it is not currently Python 2/3 compatible. This is due to two
>>>> things:
>>>
>>> Cool! Thank you for taking your time to review the work.
>>>
>>>> 1. The way iterators are implemented in Python 2/3 is different. This
>>>> problem could be remedied by adding a method to
>>>> ``PythonGraphTraversal``, something like:
>>>>
>>>> def __next__(self):
>>>> return self.next()
>>>>
>>>> then, in the ``next`` method, changing line 113 to
>>>> ``next(self.results)``, should take care of this problem.
>>>
>>> Updated.
>>>
>>>> 2. The ``GroovyTranslator`` class checks for type ``long``, this no
>>>> longer exists in Python 3. Determining what to submit as a Long might
>>>> require some discussion, but this could be easily fixed by adding
>>>> something like:
>>>>
>>>> import sys
>>>> if sys.version_info.major > 2:
>>>> long = int
>>>>
>>>> to the top of the file.
>>>
>>> Updated.
>>>
>>>>
>>>> Other than this, there are some minor details that could be cleaned
>>>> up. Particularly:
>>>>
>>>> 1. Using ``isinstance`` instead of ``type`` to perform type checking.
>>>> This will recognize subclasses and is the most Pythonic way to do
>>>> this.
>>>
>>> Seems isinstance() isn’t a method on primitives — only objects. Thus, the
>>> code got complex. Left it with type().
>>>
>>>>
>>>> 2. Formatting - indents, and line spacing. Typically, Python methods
>>>> are separated by a single line, and indents use four spaces. This is
>>>> really just cosmetic for readability.
>>>
>>> Uh. The problem is that the source code is auto-generated from
>>> GremlinPythonGenerator. If you want to tweak, please do so:
>>>
>>>
>>> https://github.com/apache/tinkerpop/blob/TINKERPOP-1278/gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy
>>> <
>>> https://github.com/apache/tinkerpop/blob/TINKERPOP-1278/gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy
>>>>
>>>
>>>
>>>> 3. CamelCase vs. underscores. I understand that to emulate Gremlin,
>>>> the traversal methods etc. should use CamelCase. But I wonder if the
>>>> helper classes (Translators) should use the Python convention of using
>>>> underscores to name methods. Python class names use camel case by
>>>> convention.
>>>
>>>
>>> I haven’t changed it. If you feel we should, please do. And yes, I think
>>> its good to keep the Gremlin step names camelCase. For Gremlin-Ruby, we
>>> will do out_e() style.
>>>
>>>> Finally, the implementation of the B class may need some work, but
>>>> we'll have to play around with it a bit to figure out how the best
>>>> approach to doing this.
>>>
>>>
>>> Yea, thats all wrong. I overdosed on the introspection and then Kuppitz
>>> was like “Why not just have it as….” (something much simpler — which I
>>> forget what it was now).
>>>
>>>> I'm sure there are more improvements, but I just wanted to get a
>>>> conversation going. I would be happy to make a PR with some of these
>>>> changes.
>>>
>>> That’d be awesome. Its in TINKERPOP-1278 branch. In gremlin-variant/test
>>> you will see PythonProcessStandardTest and PythonProcessComputerTest. Those
>>> verify that the compilation is valid for all the standard and computer
>>> tests.
>>>
>>>> Also, gremlinclient will soon support the RemoteConnection interface,
>>>> I'll send out a link to the docs once I get everything up and running.
>>>
>>> So cool. Please do review the Python RemoteConnection class as again, I
>>> just improv’d it.
>>>
>>> Thanks again David,
>>> Marko.
>>>
>>> http://markorodriguez.com
>
>
>
> --
> David M. Brown
> R.A. CulturePlex Lab, Western University
Re: gremlin_python GLV
Posted by Marko Rodriguez <ok...@gmail.com>.
Hi,
That is a really good idea.
Want to do a PR to the branch or do you want me to just take your notes/gist and make it happen?
Marko.
http://markorodriguez.com
> On Jun 15, 2016, at 3:55 PM, Leifur Halldor Asgeirsson <LA...@zerofail.com> wrote:
>
> Hi all,
> I have a couple of suggestions for the GLV.
>
> Firstly, it would be useful to be able to inject arbitrary expressions, such as static constructors. For example, in an application that I'm currently working on, I use Titan's Geoshape property type. I would like to be able to call one of the static constructors on the Geoshape class in a script, passing it bound parameters. Something like
>
> "g.V().has('location', geoWithin(Geoshape.circle(lat, lon, radius)))"
> with bound parameters {'lat': 45, 'lon': 45, 'radius': '10'}
>
> I took an initial stab at this today, and this is what I came up with:
> https://gist.github.com/leifurhauks/5a843379183123dbed1134a92691a335
>
> With those changes, I can use that static constructor in a traversal as follows:
>
>>>> translator = GroovyTranslator('g')
>>>> g = PythonGraphTraversalSource(translator)
>>>> t = g.V().has('location', RawExpression('Geoshape.point(', B('lat', 45), ', ', B('lon', 45), ')'))
>>>> str(t)
> 'g.V().has("location", Geoshape.point(lat, lon))'
>>>> t.bindings
> {'lon': 45, 'lat': 45}
>
> That raw expression isn't very readable, but a simple helper class can fix that:
>
> class Geoshape(object):
> @staticmethod
> def point(latitude, longitude, symbols=('lat', 'lon')):
> return RawExpression(
> 'Geoshape.point(', B(symbols[0], latitude), ', ', B(symbols[1], longitude), ')')
>
> Now I can rewrite the previous traversal to be much clearer:
>
> t = g.V().has('location', Geoshape.point(45, 45))
>
>
> If this seems like a reasonable approach, I would be happy to submit a PR.
>
>
> I have one other suggestion, but this one is tiny. Because most of the steps on PythonGraphTraversal have the same implementation, it would be possible to specify that implementation once in a function factory, like this:
>
> def simple_step(name):
> def step_method(self, *args):
> self.translator.addStep(self, name, *args)
> for arg in args:
> if type(arg) is B:
> self.bindings[arg.symbol] = arg.value
> return self
> return step_method
>
> Then, on PythonGraphTraversal, all the step methods that use that implementation could be declared like this:
>
> class PythonGraphTraversal(object):
> def __init__(self, translator, remote_connection=None):
> # elided ...
> # top methods elided
> has = simple_step('has')
> hasId = simple_step('hasId')
> hasKey = smple_step('hasKey')
> # and so on...
>
>
> ________________________________________
> From: David Brown <da...@gmail.com>
> Sent: June 15, 2016 3:23 PM
> To: dev@tinkerpop.apache.org
> Subject: Re: gremlin_python GLV
>
> Ok Stephen I will. I can't claim to be an expert (especially when it
> comes to the Java ecosystem), but I will definitely take a look.
>
> Also, Marko, Python doesn't really have primitives as such. The built
> in function isinstance should work for everything in gremlin_python
> e.g. ``isinstance(var, bool)``. I can take a look at this when I make
> a PR if you'd like, but it really isn't a huge deal anyway.
>
> On Wed, Jun 15, 2016 at 3:16 PM, Stephen Mallette <sp...@gmail.com> wrote:
>> David, it would also be great to get your feedback on the
>> packaging/deployment approach we have so far. I have some basic figured out
>> for deployment to pypi over maven via twine, but it could use an experts
>> eye. You probably don't need to check that part out now as you are already
>> have some other stuff to look at, but I just wanted to mention that so that
>> it was in your mind for later. Thanks for your help on this.
>>
>> On Wed, Jun 15, 2016 at 3:02 PM, Marko Rodriguez <ok...@gmail.com>
>> wrote:
>>
>>> Hello,
>>>
>>>> I was reading through the gremlin_python GLV code this morning, and
>>>> overall it looks like it should work pretty smoothly. Thanks for doing
>>>> this Marko, really cool work! I just wanted to comment on a few things
>>>> that popped out at me on my first reading.The major issue I see is
>>>> that it is not currently Python 2/3 compatible. This is due to two
>>>> things:
>>>
>>> Cool! Thank you for taking your time to review the work.
>>>
>>>> 1. The way iterators are implemented in Python 2/3 is different. This
>>>> problem could be remedied by adding a method to
>>>> ``PythonGraphTraversal``, something like:
>>>>
>>>> def __next__(self):
>>>> return self.next()
>>>>
>>>> then, in the ``next`` method, changing line 113 to
>>>> ``next(self.results)``, should take care of this problem.
>>>
>>> Updated.
>>>
>>>> 2. The ``GroovyTranslator`` class checks for type ``long``, this no
>>>> longer exists in Python 3. Determining what to submit as a Long might
>>>> require some discussion, but this could be easily fixed by adding
>>>> something like:
>>>>
>>>> import sys
>>>> if sys.version_info.major > 2:
>>>> long = int
>>>>
>>>> to the top of the file.
>>>
>>> Updated.
>>>
>>>>
>>>> Other than this, there are some minor details that could be cleaned
>>>> up. Particularly:
>>>>
>>>> 1. Using ``isinstance`` instead of ``type`` to perform type checking.
>>>> This will recognize subclasses and is the most Pythonic way to do
>>>> this.
>>>
>>> Seems isinstance() isn’t a method on primitives — only objects. Thus, the
>>> code got complex. Left it with type().
>>>
>>>>
>>>> 2. Formatting - indents, and line spacing. Typically, Python methods
>>>> are separated by a single line, and indents use four spaces. This is
>>>> really just cosmetic for readability.
>>>
>>> Uh. The problem is that the source code is auto-generated from
>>> GremlinPythonGenerator. If you want to tweak, please do so:
>>>
>>>
>>> https://github.com/apache/tinkerpop/blob/TINKERPOP-1278/gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy
>>> <
>>> https://github.com/apache/tinkerpop/blob/TINKERPOP-1278/gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy
>>>>
>>>
>>>
>>>> 3. CamelCase vs. underscores. I understand that to emulate Gremlin,
>>>> the traversal methods etc. should use CamelCase. But I wonder if the
>>>> helper classes (Translators) should use the Python convention of using
>>>> underscores to name methods. Python class names use camel case by
>>>> convention.
>>>
>>>
>>> I haven’t changed it. If you feel we should, please do. And yes, I think
>>> its good to keep the Gremlin step names camelCase. For Gremlin-Ruby, we
>>> will do out_e() style.
>>>
>>>> Finally, the implementation of the B class may need some work, but
>>>> we'll have to play around with it a bit to figure out how the best
>>>> approach to doing this.
>>>
>>>
>>> Yea, thats all wrong. I overdosed on the introspection and then Kuppitz
>>> was like “Why not just have it as….” (something much simpler — which I
>>> forget what it was now).
>>>
>>>> I'm sure there are more improvements, but I just wanted to get a
>>>> conversation going. I would be happy to make a PR with some of these
>>>> changes.
>>>
>>> That’d be awesome. Its in TINKERPOP-1278 branch. In gremlin-variant/test
>>> you will see PythonProcessStandardTest and PythonProcessComputerTest. Those
>>> verify that the compilation is valid for all the standard and computer
>>> tests.
>>>
>>>> Also, gremlinclient will soon support the RemoteConnection interface,
>>>> I'll send out a link to the docs once I get everything up and running.
>>>
>>> So cool. Please do review the Python RemoteConnection class as again, I
>>> just improv’d it.
>>>
>>> Thanks again David,
>>> Marko.
>>>
>>> http://markorodriguez.com
>
>
>
> --
> David M. Brown
> R.A. CulturePlex Lab, Western University
Re: gremlin_python GLV
Posted by Leifur Halldor Asgeirsson <LA...@zerofail.com>.
Hi all,
I have a couple of suggestions for the GLV.
Firstly, it would be useful to be able to inject arbitrary expressions, such as static constructors. For example, in an application that I'm currently working on, I use Titan's Geoshape property type. I would like to be able to call one of the static constructors on the Geoshape class in a script, passing it bound parameters. Something like
"g.V().has('location', geoWithin(Geoshape.circle(lat, lon, radius)))"
with bound parameters {'lat': 45, 'lon': 45, 'radius': '10'}
I took an initial stab at this today, and this is what I came up with:
https://gist.github.com/leifurhauks/5a843379183123dbed1134a92691a335
With those changes, I can use that static constructor in a traversal as follows:
>>> translator = GroovyTranslator('g')
>>> g = PythonGraphTraversalSource(translator)
>>> t = g.V().has('location', RawExpression('Geoshape.point(', B('lat', 45), ', ', B('lon', 45), ')'))
>>> str(t)
'g.V().has("location", Geoshape.point(lat, lon))'
>>> t.bindings
{'lon': 45, 'lat': 45}
That raw expression isn't very readable, but a simple helper class can fix that:
class Geoshape(object):
@staticmethod
def point(latitude, longitude, symbols=('lat', 'lon')):
return RawExpression(
'Geoshape.point(', B(symbols[0], latitude), ', ', B(symbols[1], longitude), ')')
Now I can rewrite the previous traversal to be much clearer:
t = g.V().has('location', Geoshape.point(45, 45))
If this seems like a reasonable approach, I would be happy to submit a PR.
I have one other suggestion, but this one is tiny. Because most of the steps on PythonGraphTraversal have the same implementation, it would be possible to specify that implementation once in a function factory, like this:
def simple_step(name):
def step_method(self, *args):
self.translator.addStep(self, name, *args)
for arg in args:
if type(arg) is B:
self.bindings[arg.symbol] = arg.value
return self
return step_method
Then, on PythonGraphTraversal, all the step methods that use that implementation could be declared like this:
class PythonGraphTraversal(object):
def __init__(self, translator, remote_connection=None):
# elided ...
# top methods elided
has = simple_step('has')
hasId = simple_step('hasId')
hasKey = smple_step('hasKey')
# and so on...
________________________________________
From: David Brown <da...@gmail.com>
Sent: June 15, 2016 3:23 PM
To: dev@tinkerpop.apache.org
Subject: Re: gremlin_python GLV
Ok Stephen I will. I can't claim to be an expert (especially when it
comes to the Java ecosystem), but I will definitely take a look.
Also, Marko, Python doesn't really have primitives as such. The built
in function isinstance should work for everything in gremlin_python
e.g. ``isinstance(var, bool)``. I can take a look at this when I make
a PR if you'd like, but it really isn't a huge deal anyway.
On Wed, Jun 15, 2016 at 3:16 PM, Stephen Mallette <sp...@gmail.com> wrote:
> David, it would also be great to get your feedback on the
> packaging/deployment approach we have so far. I have some basic figured out
> for deployment to pypi over maven via twine, but it could use an experts
> eye. You probably don't need to check that part out now as you are already
> have some other stuff to look at, but I just wanted to mention that so that
> it was in your mind for later. Thanks for your help on this.
>
> On Wed, Jun 15, 2016 at 3:02 PM, Marko Rodriguez <ok...@gmail.com>
> wrote:
>
>> Hello,
>>
>> > I was reading through the gremlin_python GLV code this morning, and
>> > overall it looks like it should work pretty smoothly. Thanks for doing
>> > this Marko, really cool work! I just wanted to comment on a few things
>> > that popped out at me on my first reading.The major issue I see is
>> > that it is not currently Python 2/3 compatible. This is due to two
>> > things:
>>
>> Cool! Thank you for taking your time to review the work.
>>
>> > 1. The way iterators are implemented in Python 2/3 is different. This
>> > problem could be remedied by adding a method to
>> > ``PythonGraphTraversal``, something like:
>> >
>> > def __next__(self):
>> > return self.next()
>> >
>> > then, in the ``next`` method, changing line 113 to
>> > ``next(self.results)``, should take care of this problem.
>>
>> Updated.
>>
>> > 2. The ``GroovyTranslator`` class checks for type ``long``, this no
>> > longer exists in Python 3. Determining what to submit as a Long might
>> > require some discussion, but this could be easily fixed by adding
>> > something like:
>> >
>> > import sys
>> > if sys.version_info.major > 2:
>> > long = int
>> >
>> > to the top of the file.
>>
>> Updated.
>>
>> >
>> > Other than this, there are some minor details that could be cleaned
>> > up. Particularly:
>> >
>> > 1. Using ``isinstance`` instead of ``type`` to perform type checking.
>> > This will recognize subclasses and is the most Pythonic way to do
>> > this.
>>
>> Seems isinstance() isn’t a method on primitives — only objects. Thus, the
>> code got complex. Left it with type().
>>
>> >
>> > 2. Formatting - indents, and line spacing. Typically, Python methods
>> > are separated by a single line, and indents use four spaces. This is
>> > really just cosmetic for readability.
>>
>> Uh. The problem is that the source code is auto-generated from
>> GremlinPythonGenerator. If you want to tweak, please do so:
>>
>>
>> https://github.com/apache/tinkerpop/blob/TINKERPOP-1278/gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy
>> <
>> https://github.com/apache/tinkerpop/blob/TINKERPOP-1278/gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy
>> >
>>
>>
>> > 3. CamelCase vs. underscores. I understand that to emulate Gremlin,
>> > the traversal methods etc. should use CamelCase. But I wonder if the
>> > helper classes (Translators) should use the Python convention of using
>> > underscores to name methods. Python class names use camel case by
>> > convention.
>>
>>
>> I haven’t changed it. If you feel we should, please do. And yes, I think
>> its good to keep the Gremlin step names camelCase. For Gremlin-Ruby, we
>> will do out_e() style.
>>
>> > Finally, the implementation of the B class may need some work, but
>> > we'll have to play around with it a bit to figure out how the best
>> > approach to doing this.
>>
>>
>> Yea, thats all wrong. I overdosed on the introspection and then Kuppitz
>> was like “Why not just have it as….” (something much simpler — which I
>> forget what it was now).
>>
>> > I'm sure there are more improvements, but I just wanted to get a
>> > conversation going. I would be happy to make a PR with some of these
>> > changes.
>>
>> That’d be awesome. Its in TINKERPOP-1278 branch. In gremlin-variant/test
>> you will see PythonProcessStandardTest and PythonProcessComputerTest. Those
>> verify that the compilation is valid for all the standard and computer
>> tests.
>>
>> > Also, gremlinclient will soon support the RemoteConnection interface,
>> > I'll send out a link to the docs once I get everything up and running.
>>
>> So cool. Please do review the Python RemoteConnection class as again, I
>> just improv’d it.
>>
>> Thanks again David,
>> Marko.
>>
>> http://markorodriguez.com
--
David M. Brown
R.A. CulturePlex Lab, Western University
Re: gremlin_python GLV
Posted by David Brown <da...@gmail.com>.
Ok Stephen I will. I can't claim to be an expert (especially when it
comes to the Java ecosystem), but I will definitely take a look.
Also, Marko, Python doesn't really have primitives as such. The built
in function isinstance should work for everything in gremlin_python
e.g. ``isinstance(var, bool)``. I can take a look at this when I make
a PR if you'd like, but it really isn't a huge deal anyway.
On Wed, Jun 15, 2016 at 3:16 PM, Stephen Mallette <sp...@gmail.com> wrote:
> David, it would also be great to get your feedback on the
> packaging/deployment approach we have so far. I have some basic figured out
> for deployment to pypi over maven via twine, but it could use an experts
> eye. You probably don't need to check that part out now as you are already
> have some other stuff to look at, but I just wanted to mention that so that
> it was in your mind for later. Thanks for your help on this.
>
> On Wed, Jun 15, 2016 at 3:02 PM, Marko Rodriguez <ok...@gmail.com>
> wrote:
>
>> Hello,
>>
>> > I was reading through the gremlin_python GLV code this morning, and
>> > overall it looks like it should work pretty smoothly. Thanks for doing
>> > this Marko, really cool work! I just wanted to comment on a few things
>> > that popped out at me on my first reading.The major issue I see is
>> > that it is not currently Python 2/3 compatible. This is due to two
>> > things:
>>
>> Cool! Thank you for taking your time to review the work.
>>
>> > 1. The way iterators are implemented in Python 2/3 is different. This
>> > problem could be remedied by adding a method to
>> > ``PythonGraphTraversal``, something like:
>> >
>> > def __next__(self):
>> > return self.next()
>> >
>> > then, in the ``next`` method, changing line 113 to
>> > ``next(self.results)``, should take care of this problem.
>>
>> Updated.
>>
>> > 2. The ``GroovyTranslator`` class checks for type ``long``, this no
>> > longer exists in Python 3. Determining what to submit as a Long might
>> > require some discussion, but this could be easily fixed by adding
>> > something like:
>> >
>> > import sys
>> > if sys.version_info.major > 2:
>> > long = int
>> >
>> > to the top of the file.
>>
>> Updated.
>>
>> >
>> > Other than this, there are some minor details that could be cleaned
>> > up. Particularly:
>> >
>> > 1. Using ``isinstance`` instead of ``type`` to perform type checking.
>> > This will recognize subclasses and is the most Pythonic way to do
>> > this.
>>
>> Seems isinstance() isn’t a method on primitives — only objects. Thus, the
>> code got complex. Left it with type().
>>
>> >
>> > 2. Formatting - indents, and line spacing. Typically, Python methods
>> > are separated by a single line, and indents use four spaces. This is
>> > really just cosmetic for readability.
>>
>> Uh. The problem is that the source code is auto-generated from
>> GremlinPythonGenerator. If you want to tweak, please do so:
>>
>>
>> https://github.com/apache/tinkerpop/blob/TINKERPOP-1278/gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy
>> <
>> https://github.com/apache/tinkerpop/blob/TINKERPOP-1278/gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy
>> >
>>
>>
>> > 3. CamelCase vs. underscores. I understand that to emulate Gremlin,
>> > the traversal methods etc. should use CamelCase. But I wonder if the
>> > helper classes (Translators) should use the Python convention of using
>> > underscores to name methods. Python class names use camel case by
>> > convention.
>>
>>
>> I haven’t changed it. If you feel we should, please do. And yes, I think
>> its good to keep the Gremlin step names camelCase. For Gremlin-Ruby, we
>> will do out_e() style.
>>
>> > Finally, the implementation of the B class may need some work, but
>> > we'll have to play around with it a bit to figure out how the best
>> > approach to doing this.
>>
>>
>> Yea, thats all wrong. I overdosed on the introspection and then Kuppitz
>> was like “Why not just have it as….” (something much simpler — which I
>> forget what it was now).
>>
>> > I'm sure there are more improvements, but I just wanted to get a
>> > conversation going. I would be happy to make a PR with some of these
>> > changes.
>>
>> That’d be awesome. Its in TINKERPOP-1278 branch. In gremlin-variant/test
>> you will see PythonProcessStandardTest and PythonProcessComputerTest. Those
>> verify that the compilation is valid for all the standard and computer
>> tests.
>>
>> > Also, gremlinclient will soon support the RemoteConnection interface,
>> > I'll send out a link to the docs once I get everything up and running.
>>
>> So cool. Please do review the Python RemoteConnection class as again, I
>> just improv’d it.
>>
>> Thanks again David,
>> Marko.
>>
>> http://markorodriguez.com
--
David M. Brown
R.A. CulturePlex Lab, Western University
Re: gremlin_python GLV
Posted by Stephen Mallette <sp...@gmail.com>.
David, it would also be great to get your feedback on the
packaging/deployment approach we have so far. I have some basic figured out
for deployment to pypi over maven via twine, but it could use an experts
eye. You probably don't need to check that part out now as you are already
have some other stuff to look at, but I just wanted to mention that so that
it was in your mind for later. Thanks for your help on this.
On Wed, Jun 15, 2016 at 3:02 PM, Marko Rodriguez <ok...@gmail.com>
wrote:
> Hello,
>
> > I was reading through the gremlin_python GLV code this morning, and
> > overall it looks like it should work pretty smoothly. Thanks for doing
> > this Marko, really cool work! I just wanted to comment on a few things
> > that popped out at me on my first reading.The major issue I see is
> > that it is not currently Python 2/3 compatible. This is due to two
> > things:
>
> Cool! Thank you for taking your time to review the work.
>
> > 1. The way iterators are implemented in Python 2/3 is different. This
> > problem could be remedied by adding a method to
> > ``PythonGraphTraversal``, something like:
> >
> > def __next__(self):
> > return self.next()
> >
> > then, in the ``next`` method, changing line 113 to
> > ``next(self.results)``, should take care of this problem.
>
> Updated.
>
> > 2. The ``GroovyTranslator`` class checks for type ``long``, this no
> > longer exists in Python 3. Determining what to submit as a Long might
> > require some discussion, but this could be easily fixed by adding
> > something like:
> >
> > import sys
> > if sys.version_info.major > 2:
> > long = int
> >
> > to the top of the file.
>
> Updated.
>
> >
> > Other than this, there are some minor details that could be cleaned
> > up. Particularly:
> >
> > 1. Using ``isinstance`` instead of ``type`` to perform type checking.
> > This will recognize subclasses and is the most Pythonic way to do
> > this.
>
> Seems isinstance() isn’t a method on primitives — only objects. Thus, the
> code got complex. Left it with type().
>
> >
> > 2. Formatting - indents, and line spacing. Typically, Python methods
> > are separated by a single line, and indents use four spaces. This is
> > really just cosmetic for readability.
>
> Uh. The problem is that the source code is auto-generated from
> GremlinPythonGenerator. If you want to tweak, please do so:
>
>
> https://github.com/apache/tinkerpop/blob/TINKERPOP-1278/gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy
> <
> https://github.com/apache/tinkerpop/blob/TINKERPOP-1278/gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy
> >
>
>
> > 3. CamelCase vs. underscores. I understand that to emulate Gremlin,
> > the traversal methods etc. should use CamelCase. But I wonder if the
> > helper classes (Translators) should use the Python convention of using
> > underscores to name methods. Python class names use camel case by
> > convention.
>
>
> I haven’t changed it. If you feel we should, please do. And yes, I think
> its good to keep the Gremlin step names camelCase. For Gremlin-Ruby, we
> will do out_e() style.
>
> > Finally, the implementation of the B class may need some work, but
> > we'll have to play around with it a bit to figure out how the best
> > approach to doing this.
>
>
> Yea, thats all wrong. I overdosed on the introspection and then Kuppitz
> was like “Why not just have it as….” (something much simpler — which I
> forget what it was now).
>
> > I'm sure there are more improvements, but I just wanted to get a
> > conversation going. I would be happy to make a PR with some of these
> > changes.
>
> That’d be awesome. Its in TINKERPOP-1278 branch. In gremlin-variant/test
> you will see PythonProcessStandardTest and PythonProcessComputerTest. Those
> verify that the compilation is valid for all the standard and computer
> tests.
>
> > Also, gremlinclient will soon support the RemoteConnection interface,
> > I'll send out a link to the docs once I get everything up and running.
>
> So cool. Please do review the Python RemoteConnection class as again, I
> just improv’d it.
>
> Thanks again David,
> Marko.
>
> http://markorodriguez.com
Re: gremlin_python GLV
Posted by Marko Rodriguez <ok...@gmail.com>.
Hello,
> I was reading through the gremlin_python GLV code this morning, and
> overall it looks like it should work pretty smoothly. Thanks for doing
> this Marko, really cool work! I just wanted to comment on a few things
> that popped out at me on my first reading.The major issue I see is
> that it is not currently Python 2/3 compatible. This is due to two
> things:
Cool! Thank you for taking your time to review the work.
> 1. The way iterators are implemented in Python 2/3 is different. This
> problem could be remedied by adding a method to
> ``PythonGraphTraversal``, something like:
>
> def __next__(self):
> return self.next()
>
> then, in the ``next`` method, changing line 113 to
> ``next(self.results)``, should take care of this problem.
Updated.
> 2. The ``GroovyTranslator`` class checks for type ``long``, this no
> longer exists in Python 3. Determining what to submit as a Long might
> require some discussion, but this could be easily fixed by adding
> something like:
>
> import sys
> if sys.version_info.major > 2:
> long = int
>
> to the top of the file.
Updated.
>
> Other than this, there are some minor details that could be cleaned
> up. Particularly:
>
> 1. Using ``isinstance`` instead of ``type`` to perform type checking.
> This will recognize subclasses and is the most Pythonic way to do
> this.
Seems isinstance() isn’t a method on primitives — only objects. Thus, the code got complex. Left it with type().
>
> 2. Formatting - indents, and line spacing. Typically, Python methods
> are separated by a single line, and indents use four spaces. This is
> really just cosmetic for readability.
Uh. The problem is that the source code is auto-generated from GremlinPythonGenerator. If you want to tweak, please do so:
https://github.com/apache/tinkerpop/blob/TINKERPOP-1278/gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy <https://github.com/apache/tinkerpop/blob/TINKERPOP-1278/gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy>
> 3. CamelCase vs. underscores. I understand that to emulate Gremlin,
> the traversal methods etc. should use CamelCase. But I wonder if the
> helper classes (Translators) should use the Python convention of using
> underscores to name methods. Python class names use camel case by
> convention.
I haven’t changed it. If you feel we should, please do. And yes, I think its good to keep the Gremlin step names camelCase. For Gremlin-Ruby, we will do out_e() style.
> Finally, the implementation of the B class may need some work, but
> we'll have to play around with it a bit to figure out how the best
> approach to doing this.
Yea, thats all wrong. I overdosed on the introspection and then Kuppitz was like “Why not just have it as….” (something much simpler — which I forget what it was now).
> I'm sure there are more improvements, but I just wanted to get a
> conversation going. I would be happy to make a PR with some of these
> changes.
That’d be awesome. Its in TINKERPOP-1278 branch. In gremlin-variant/test you will see PythonProcessStandardTest and PythonProcessComputerTest. Those verify that the compilation is valid for all the standard and computer tests.
> Also, gremlinclient will soon support the RemoteConnection interface,
> I'll send out a link to the docs once I get everything up and running.
So cool. Please do review the Python RemoteConnection class as again, I just improv’d it.
Thanks again David,
Marko.
http://markorodriguez.com