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

Remove mentions of the Id type #1005

Merged
merged 1 commit into from
Sep 14, 2023
Merged

Remove mentions of the Id type #1005

merged 1 commit into from
Sep 14, 2023

Conversation

plt-amy
Copy link
Member

@plt-amy plt-amy commented May 15, 2023

With the addition of indexed inductive types to Cubical Agda, having the built-in cubical identity type is redundant, since it has exactly the same computational behaviour as the indexed equality. However, cubical identity has its own support code for pattern matching, the Kan operations, and its magic type checking rule, which is now for no benefit. This PR removes the mentions of it from the library so I can remove it from Agda.

@plt-amy plt-amy requested a review from ecavallo May 15, 2023 13:42
@maxsnew
Copy link
Collaborator

maxsnew commented May 17, 2023

Is there somewhere I can read about this change? I just started using Cubical.Foundations.Id in a development so I don't understand how I am supposed to update my code

@JonasHoefer
Copy link
Contributor

JonasHoefer commented Jun 12, 2023

@maxsnew The inductively defined equality type in Cubical.Data.Equality has the same computational behavior as the one in Cubical.Foundations.Id and mostly the same interface. Furthermore, #1011 extends the module Cubical.Data.Equality slightly.

Copy link
Collaborator

@mortberg mortberg left a comment

Choose a reason for hiding this comment

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

I think this seems reasonable. What does @ecavallo think ? If you're happy feel free to merge

@ecavallo
Copy link
Collaborator

OK by me

@felixwellen
Copy link
Collaborator

Did you notice the conflicts?
Some large part of the issue addressed here was resolved in a different way in #1011

@felixwellen
Copy link
Collaborator

Something that was done here, but not in #1011 is deletion of things like Cubical.Core.Id, which I guess, would be good.

@mortberg
Copy link
Collaborator

Oops, I missed that.. Can @plt-amy update the PR to fix the conflicts and then we can merge?

@plt-amy
Copy link
Member Author

plt-amy commented Sep 13, 2023

Sure, I'll have a look later today 🙂

@mortberg
Copy link
Collaborator

Looks good to me! Is this ready to be merged?

@felixwellen
Copy link
Collaborator

Looks good (and ready) to me

@mortberg mortberg merged commit f697467 into master Sep 14, 2023
1 check passed
@ice1000 ice1000 deleted the aliao/remove-cubical-id branch October 24, 2023 15:07
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

6 participants