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
<[email protected]> wrote:
> Thanks! I'll submit my PR in the morning.
>
> ________________________________________
> From: Marko Rodriguez <[email protected]>
> Sent: June 15, 2016 6:13 PM
> To: [email protected]
> 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 
>> <[email protected]> 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 <[email protected]>
>> Sent: June 15, 2016 3:23 PM
>> To: [email protected]
>> 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 <[email protected]> 
>> 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 <[email protected]>
>>> 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

Reply via email to