-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add By modulators for select steps #241
base: master
Are you sure you want to change the base?
Conversation
Oh, and the calling syntax for the "pair-wise" signature would be something like this:
|
It's worth mentioning that I also considered an argument list of TupleN[SelectBy*] where SelectBy is defined as something like this:
That would give the "identity" By() modulator a bit more type safety, as it would force the Modulated type to the StepLabel type. However, I was leery of adding more types to the public interface. |
I'll definitely get back to you on this, since I had some thoughts on this topic, but I'm super busy at the moment, so please forgive the waiting time. |
This is awesome, it's taking the already quite cool It's so rewarding to see this stuff actually work. And we're almost there...
Specifying a random return type is my way of debugging the compiler, it tells me the actual return type in the error msg :)
|
I'm getting closer, not sure if the value level works yet, and need to clean up:
|
Great progress! I haven't had a chance to get back into this since last week. Have you given any thought to using a trait for each element (like the SelectBy), instead of a (StepLabel[],By[])? It's further from the gremlin idiom, but it provides a bit more type safety. It may be possible to support both the tupled version or SelectBy in one method, by making SelectBy extend Product, or by making it a case class. My concern with the tupled version is that the identity By() requires you to supply the output type, which is effectively a user-specified cast. It doesn't do much to help By(T(oken)), other than the T.label version where we could coerce that into String. Having said that, I also like the API cleanliness of sticking with more familiar gremlin constructs. |
Yes, I'm still a bit undecided, will see how it plays out. Might also be useful for the (common) case that someone only wants to by-modulate a subset of the selected values, but maybe we can handle that with the tupled version as well. |
Here's a cut at adding by modulation for select steps. Please take a look at the SelectSpec for usage of the tupled version. It accepts a tuple of StepLabel, followed by a tuple of By. The method signature for select/by could alternately be expressed as a Tuple of (StepLabel,By) pairs. I didn't spend terribly much time with it, but during my first pass at the pair-wise signature, I struggled to unzip the pairs HLists into an HList of StepLabel and an HList of Bys, because I'm not deep enough into shapeless to provide it with an implicit Unzip.Aux that the compiler could resolve. Before I hop into that rabbit hole, I thought I'd inquire with you about the method signature you would prefer to see for select/by (either one I've proposed or an alternate). Also, if you have any advice on how I could augment the following to supply an unzipper, please let me know.
That fails to compile with:
Thank you for your efforts on this library!