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

Migrate _↔̇_ to the new Function hierarchy #1978

Closed
wants to merge 1 commit into from

Conversation

Saransh-cpp
Copy link
Contributor

@Saransh-cpp Saransh-cpp commented Jun 12, 2023

Xref #759

The issue currently does not mention a module where _↔̇_ should be moved to, but given that _↔_ was moved from Function.Inverse to Function.Bundles._, I have moved _↔̇_ in a similar fashion as well.

Edit: Oops, GitHub is automatically converting _↔̇_ with no backticks to ↔̇

@Saransh-cpp Saransh-cpp changed the title Migrate _↔̇_ to the new Function hierarchy Migrate _↔̇_ to the new Function hierarchy Jun 12, 2023
@Taneb
Copy link
Member

Taneb commented Jun 12, 2023

I'm not familiar with _↔̇_. This isn't really relevant to reviewing this PR but could someone explain to me what it's meant to represent and be used for?

@Saransh-cpp
Copy link
Contributor Author

I couldn't figure out its usage too 🙂

@MatthewDaggitt
Copy link
Contributor

Hmm so it looks like it's lifting the Inverse relation to indexed sets. I guess therefore it should really go in a new module Function.Indexed.Bundles. It'll be a bit lonely in there but eventually the module might get fleshed out.

Any chance you can move it there @Saransh-cpp ? If you were keen then you could add the equivalents of the other arrows below there too?

infix 3 _⟶_ _↣_ _↠_ _⤖_ _⇔_ _↩_ _↪_ _↩↪_ _↔_

I think we should probably rename the arrow though to follow the naming conventions of the other indexed types as well as making it extendable, as I doubt the other arrow types have dotted variants. We should follow the convention below and use a subscript i instead:

therefore the name would change from _↔̇_ to _↔ᵢ_

@MatthewDaggitt
Copy link
Contributor

Also very minor point, but could you avoid using unicode characters in branch names? It messes with some Github systems as well as being very difficult to type outside emacs 👍

@Saransh-cpp Saransh-cpp deleted the migrate_↔̇_ branch June 14, 2023 13:37
@Saransh-cpp
Copy link
Contributor Author

Woops, renaming the branch closed the PR, did not know that.

Thank you for the detailed comment! I'll add equivalents of the other arrows in the file too and rename the existing one to a better name.

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