-
Notifications
You must be signed in to change notification settings - Fork 52
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 affine traversals #112
Add affine traversals #112
Conversation
Sorry for the ugly duplication with At, maybe we want to put them in the same module? Then index i = at i <<< _Just
I'm happy to help merge and release once @MonoidMusician and @LiamGoodacre are able to weigh in on the changes themselves. As a note on the build -- I'm going to be updating all the |
Thanks for this, it looks good to me! Sorry for letting the other PR stall. My two minor comments are (1) maybe you want to provide a simple version of |
(1) This leads to the question how to call it? Also I don't think people will often implement affine traversals by hand, so there's not much need to make that easier. (2) There's not much shared code anymore between |
I’d like to leave this open for another few days for @LiamGoodacre or other maintainers to weigh in: if there are no objections in that time I will merge this. Thanks for your work! |
This is an attempt at reviving PR #80.
Thanks to @MonoidMusician and @LiamGoodacre for most of the code.
What I changed is using
affineTraversal
for most of theix
implementations and replacingmaybe
withmaybe'
to prevent unnecessary calculations inData.Lens.At
(as noted by @LiamGoodacre in #80).