-
Notifications
You must be signed in to change notification settings - Fork 707
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
Make it easier to get around Tuple22 restrictions with Shapeless/HLists #1236
base: develop
Are you sure you want to change the base?
Conversation
) ++ ( | ||
if(isScala210x(scalaVersion.value)) { | ||
Seq( | ||
"com.chuusai" % "shapeless_2.10.4" % shapelessVersion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this actually compatible with 2.10.3 -> 2.10.5?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding (based on the README) is that shapeless 2.1.0 is only compatible with 2.11.x and 2.10.4, but I might be mistaken. I think the relevant bits are as follows:
Builds are available for Scala 2.11.x and for Scala 2.10.4
...
Note that for Scala 2.10.4 you must provide an explicit Scala version suffix to your shapeless dependency, and it is recommanded to add the macro paradise plugin to your build, for some macros provided by shapeless to work smoothly
For shapeless 2.0.0, they have the following comments:
Builds are available for Scala 2.11.x and for Scala 2.10.4.
...
For Scala 2.10.x you must specify a Scala version of at least 2.10.2, and add eithercross CrossVersion.full
or provide an explicit Scala version suffix to your shapeless dependency
...
Note that Scala 2.10.x releases are compatible with each other starting from 2.10.2, so a mismatch in minor versions above would be fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shapeless 2.1.0 has full cross-versions for 2.10 starting at 2.10.4 (e.g for 2.10.5), so it'd be better to use this:
"com.chuusai" %% "shapeless" % shapelessVersion cross CrossVersion.full
So that we get the 2.10.5 version when Scalding updates to 2.10.5.
(I don't remember whether you need %%
for full cross-versioning—I think either will work, and I find it a little clearer with %%
.)
See also: #746 |
new CTuple(list:_*) | ||
} | ||
|
||
override def arity: Int = ti() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be a val, to be sure we only call once.
This is great. There are two thoughts I have:
All said, I am starting to think we don't need this as much as we used to now that we have macros to do this (and they should have no performance issues). You can make a case class for your record and get a TypeDescriptor that has all the stuff you need to flatten into cascading. |
implicit def hListConverter[H, T <: HList, N <: Nat] | ||
(implicit | ||
g: TupleGetter[H], | ||
len: Length.Aux[H :: T, N], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still getting a sense of the design here, but any reason you don't constrain the FromTupleEntry
instances so that the I
(which could be a type member) matches the length of the HList
? That should allow you to omit the N
type parameter here altogether.
See my branch here for a quick demonstration of an alternative approach. The key idea is that you don't need to track the length of the More controversially, you don't necessarily even need a separate All of the new tests pass unchanged. |
I should add that this would be a pretty fantastic start even if you weren't brand new to Shapeless, @matt-martin. |
Thanks all for the comments. Only skimmed them briefly today, but will take a closer look at them tomorrow |
I'm very happy to support things like this ... ping me if there's anything I can do to help. |
One other thing: it might be worth trying to do this via |
One thing on a code perspective, given shapelesses tighter requirements around scala versioning and adding another dep I think it would make sense to move this into a scalding-shapeless module. It'll make it easier to accept/merge. |
FWIW, from shapeless-2.2.0 I'm going to drop the specific sub-version requirements for Scala 2.10.x. |
|
This is just a very basic attempt to integrate Shapeless (specifically HLists) into Scalding. There's certainly more that could be done, but I think this pull request represents a necessary first step. I would love some feedback, since I'm basically brand new to Shapeless and did this partly as an exercise to try to learn more about it. If this looks good/useful, then I suppose the next step would be thinking about how to use extensible records like Miles did in the example he already put together.