Closed fwbrasil closed 6 years ago
@johnynek @benpence @dieu @ttim Could someone merge this? I don't have access. I'd like to move forward and submit the second PR.
@ttim I addressed your feedback on https://github.com/twitter/scalding/pull/1761
I guess the only real concern from me - Projections sounds more like Map[ParamName, Set[Projections]] and not like Set[Projection].
We could use beta reded uction to deal with projections based on the actual parameters. It doesn't seem necessary, though. It would make the code much more complex without clear benefit since the projections are aggregated at the end and it doesn't really matter from which parameter they came from.
We could use beta reded uction to deal with projections based on the actual parameters. It doesn't seem necessary, though. It would make the code much more complex without clear benefit since the projections are aggregated at the end and it doesn't really matter from which parameter they came from.
I don't think it complicates a logic anyhow significantly, but it makes part where you combine projections between function call and function body more straightforward and correct.
I don't think it complicates a logic anyhow significantly, but it makes part where you combine projections between function call and function body more straightforward and correct.
The projection logic is based on type references right now. In order to do detect which projection is from which parameter, we'd need to introduce a logic similar to beta reduction. I don't see how it'd be more correct or precise than the current implementation.
See https://github.com/twitter/scalding/pull/1754 for more details