Skip to content

Use Leibniz equality #11

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

Merged
merged 8 commits into from
Nov 26, 2020
Merged

Use Leibniz equality #11

merged 8 commits into from
Nov 26, 2020

Conversation

hdgarrood
Copy link
Contributor

Following on from #9

MonoidMusician and others added 5 commits July 24, 2020 12:12
- Rename `leibniz` to `typeEqualsProof`. I changed my mind about `proof`
  because I think it's likely to be used as a variable name in other
  places already.
- Remove `leibnizOp`
- Simplify implementations of `to` and `from`
@hdgarrood
Copy link
Contributor Author

cc @kl0tl

Copy link
Member

@kl0tl kl0tl left a comment

Choose a reason for hiding this comment

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

I would find an implementation of from based on newtype From a b = From (b -> a) a bit simpler:

from :: forall a b. TypeEquals a b => b -> a
from = case typeEqualsProof (From (\a -> a)) of From f -> f

especially if we don’t export symm.

A similar implementation of to based on a redundant newtype To a b = To (a -> b) would then be nice for symmetry.

This looks good to me regardless though!

test/Main.purs Outdated
Comment on lines 18 to 19
test2 :: forall ty row. TypeEquals row ( message :: String ) => Newtype ty (Record row) => ty -> String
test2 = unwrap >>> typeEqualsProof >>> _.message
Copy link
Member

Choose a reason for hiding this comment

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

message seems like a good name for that function.

class TypeEquals a b | a -> b, b -> a where
to :: a -> b
from :: b -> a
typeEqualsProof :: forall p. p a -> p b
Copy link
Member

Choose a reason for hiding this comment

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

Maybe just proof?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think proof is more likely to be used as a local variable name, which is why I was thinking of giving this a longer name. I guess it depends whether we expect this module to be imported qualified or not?

Copy link
Member

Choose a reason for hiding this comment

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

I was imagining it'll be qualified - that's also the pattern we've been encouraging in most places - dropping type/class names from functions and members, like all the Ref stuff a while back.

With it already having names like to and from I've only ever imported it qualified as it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, true. It seems Type.Prelude currently re-exports the entirety of this module unqualified, which I think is perhaps not ideal. Come to think of it I'm not sure if people tend to import Type.Prelude qualified or not. Maybe we could just not re-export the new proof from Type.Prelude?

Copy link
Member

Choose a reason for hiding this comment

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

Not re-exporting proof sounds fine to me 👍.

I'm not sure I've ever imported Type.Prelude at all, but if I were to do so I'd probably import it qualified, but then I import almost everything qualified these days, unless it's in our app prelude.

Copy link
Member

Choose a reason for hiding this comment

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

Type.Prelude re-exports only TypeEquals, from and to so nothing to change there then o/

@hdgarrood
Copy link
Contributor Author

If someone wants to carry this over the line I'd be grateful. I think all that's necessary is replacing typeEqualsProof with proof. I would also be happy to go with @kl0tl's suggestion of using newtypes To and From to implement to and from, or alternatively I think leaving the implementations of to and from as they are is probably fine too.

@kl0tl
Copy link
Member

kl0tl commented Nov 23, 2020

I pushed a few commits, this should be good to merge once Travis agrees.

@JordanMartinez JordanMartinez merged commit 564030a into master Nov 26, 2020
@JordanMartinez JordanMartinez deleted the leibniz branch November 26, 2020 19:21
@kl0tl kl0tl mentioned this pull request Dec 1, 2020
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.

5 participants