MikeInnes / Lazy.jl

I was gonna maintain this package, but then I got high
Other
470 stars 54 forks source link

Extend foreach when defined by Base or Compat. #39

Closed MichaeLeroy closed 8 years ago

MichaeLeroy commented 8 years ago

The generic function foreach is entering Julia Base with version 0.5 (and earlier versions via Compat). However this function is also defined in Lazy. This PR addresses the resulting conflict by importing foreach from the appropriate source so that Lazy extends foreach rather than defining it ab initio.

Caveat The type signature is the Lazy version of foreach makes this extension clear-cut and straightforward. However, Base.foreach has a different meaning as a generic function to Lazy.foreach. So it may be preferable to avoid the conflict by no longer exporting foreach rather than extending it (as was done for other conflicting identifiers for issue #27).

MikeInnes commented 8 years ago

Thanks for the patch! Can you clarify in what sense foreach has a different meaning in Base vs Lazy? They look like they would do the same thing to me (apply a function to each element and return nothing).

MichaeLeroy commented 8 years ago

@MikeInnes my apologies for any confusion caused by the caveat. I didn't read the code for Lazy.foreach carefully enough to positively assert that the Lazy version is consonant with that of Base and added the caveat to be conservative in my claims. Please feel free to ignore it and act accordingly. I'd be quite happy to have this PR accepted as is if you find it satisfactory.

MikeInnes commented 8 years ago

Ok great, just making sure – thanks again.