Skip to content
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

Linear lens combinators #195

Merged
merged 2 commits into from
Sep 22, 2020
Merged

Linear lens combinators #195

merged 2 commits into from
Sep 22, 2020

Conversation

aspiwack
Copy link
Member

A handful of utilities for linear lens.

These are ported from #79 .

@aspiwack aspiwack mentioned this pull request Sep 18, 2020
Copy link
Contributor

@Divesh-Otwani Divesh-Otwani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A simple migration 👍 -- Oh, please fix the CI before merging. No idea why it's failing. Maybe some cut-and-paste of strangely encoded characters?

Comment on lines +101 to +108
instance (Functor f, Functor g) => Functor (Compose f g) where
fmap f (Compose x) = Compose (fmap (fmap f) x)

instance (Applicative f, Applicative g) => Applicative (Compose f g) where
pure x = Compose (pure (pure x))
(Compose f) <*> (Compose x) = Compose (liftA2 (<*>) f x)
liftA2 f (Compose x) (Compose y) = Compose (liftA2 (liftA2 f) x y)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like these. I needed them for linear-streams.

@@ -67,6 +70,10 @@ assoc = iso Bifunctor.lassoc Bifunctor.rassoc
(.>) :: Optic_ arr a b s t -> Optic_ arr x y a b -> Optic_ arr x y s t
Optical f .> Optical g = Optical (f P.. g)


lens :: (s #-> (a, b #-> t)) -> Lens a b s t
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice; me likey 😏

@@ -139,3 +152,10 @@ withIso (Optical l) f = f fro to
withPrism :: Optic_ (Market a b) a b s t -> ((b #-> t) -> (s #-> Either t a) -> r) -> r
withPrism (Optical l) f = f b m
where Market b m = l (Market id Right)

withLens :: Optic_ (Linear.Kleisli (Compose ((,) a) (FUN 'One b))) a b s t -> s #-> (a, b #-> t)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that this is just migration and that I'm going to take a pass through optics, this, although complicated, is fine right now. When I take a pass I'm going to comment that withLens is (kind of) the dual of lens and the (Optic_ ...) is basically (a #-> (a, b -> b)) -> (s #-> (s, t -> t)).

One question: could we change FUN 'One b to (->) b which is much more readable to me?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

withLens is (kind of) the dual of lens

The better term is inverse

could we change FUN 'One b to (->) b which is much more readable to me?

It would not work. (->) is FUN 'Many. There are many ways to make this simpler, but I don't think that they are much needed. The type after Optic_ is not meant to be read. See the lens library for how to document such primitives.

@utdemir
Copy link
Contributor

utdemir commented Sep 18, 2020

fix the CI before merging. No idea why it's failing. Maybe some cut-and-paste of strangely encoded characters?

It's a weird issue. I had it once before (on #189), I think there's a cache invalidation issue somewhere. Previously I got around that by randomly changing a parameter name on one of the upstream modules. Totally unsatisfactory "solution", I know.

@aspiwack
Copy link
Member Author

I can reproduce the stack failure locally. With a clean .stack and .stack-work.

Stack version

Version 2.1.3, Git revision 636e3a759d51127df2b62f90772def126cdf6d1f (7735 commits) x86_64 hpack-0.31.2

I don't really have time to figure this out, right now, though.

@Divesh-Otwani
Copy link
Contributor

Okay, I'll play with parameter names and then merge.

@aspiwack
Copy link
Member Author

It does make me very sad, if that's the solution we end up settling on.

@utdemir
Copy link
Contributor

utdemir commented Sep 22, 2020

It does make me very sad, if that's the solution we end up settling on.

I agree. I spent some time trying to solve it but to no avail. So I gave up and opened #199, it's still not a solution; but should be better than having to change random stuff.

With a linear lens, we cannot linearly set the value at the lens site
in general, however, we can swap the value at the lens site for
another, which we get out of the operation.

Ported from #79.
Translation between the profunctor definition of lens and the more
straightforward getter/setter pair.

Ported from #79.
@utdemir utdemir force-pushed the linear-lens-combinators branch from 3791580 to 7b3056a Compare September 22, 2020 09:19
@utdemir utdemir merged commit 0d47132 into master Sep 22, 2020
@utdemir utdemir deleted the linear-lens-combinators branch September 22, 2020 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants