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

Add unit and sigma types #4

Merged
merged 21 commits into from
Jun 26, 2020
Merged

Add unit and sigma types #4

merged 21 commits into from
Jun 26, 2020

Conversation

Vtec234
Copy link
Contributor

@Vtec234 Vtec234 commented Jun 19, 2020

I worked on this for my master's thesis. The PR adds a contractible unit type and dependent sums with an extensionality principle.

@mr-ohman
Copy link
Owner

Thank you @Vtec234 for contributing to this project. Great work! I like how you were able to reuse the definition of Π in the logical relation and generalize it to work with both Π and Σ. The other changes and refactoring are also welcome. The only thing I found confusing was the naming choice of star for the value of Unit, I would have preferred tt or <> as those are more standard. Other than that, everything looks good.

@Vtec234
Copy link
Contributor Author

Vtec234 commented Jun 24, 2020

Thanks for your comments, I'm happy to contribute! Regarding star, I was going by the name used in the Lean standard library. I see that both Coq and Agda do use tt, so I tried changing the term to that just now. But then it causes naming conflicts with Agda's own constructor! And for me, <> only brings to mind Haskell monoids or OCaml inequality. Perhaps the slightly offbeat naming is acceptable to avoid conflicts - what do you think?

@mr-ohman
Copy link
Owner

Yes, while it is possible use tt for both simultaneously with qualified names, it may be annoying and inelegant to have to keep referring to them that way. For <> I was thinking of it as a substitute for () from Haskell and ML, but I forgot about <> being Haskell mappend and OCaml inequality. I wasn't aware of star in the Lean standard library, so I think it is fine to use that name, both to avoid conflicts as you said and since it is at least standard in one language.

Since that was my only objection, I will go ahead and merge this. Thanks again for your contribution!

@mr-ohman mr-ohman merged commit 647e306 into mr-ohman:master Jun 26, 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.

None yet

2 participants