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

Refactoring positive integers #1059

Merged
merged 117 commits into from
Mar 28, 2024
Merged

Conversation

malarbol
Copy link
Contributor

@malarbol malarbol commented Mar 6, 2024

Resolves #1043.

@malarbol malarbol marked this pull request as draft March 7, 2024 00:11
@fredrik-bakke
Copy link
Collaborator

I quite like your organization of the concepts into their own files + a file about the relations between them. But if I didn't misunderstand @EgbertRijke rather wanted one file for all 5? Maybe he can comment on that.

@fredrik-bakke
Copy link
Collaborator

Your work looks pretty good so far by the way!

Copy link
Collaborator

@fredrik-bakke fredrik-bakke left a comment

Choose a reason for hiding this comment

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

In the library, I believe "inequality" is the name we use for the standard ordering on integers and so on. We shouldn't introduce "standard inequality" as an extra term for the same thing.

@malarbol
Copy link
Contributor Author

In the library, I believe "inequality" is the name we use for the standard ordering on integers and so on. We shouldn't introduce "standard inequality" as an extra term for the same thing.

Ok. I chose inequality over order(ing) to stick with the names of the files ; maybe this was an error and I should have kept a bit of both? And I just stuck standard in a few places because of your comment on the many orderings of ℤ but we can drop them if you prefer.

@fredrik-bakke
Copy link
Collaborator

In the library, I believe "inequality" is the name we use for the standard ordering on integers and so on. We shouldn't introduce "standard inequality" as an extra term for the same thing.

Ok. I chose inequality over order(ing) to stick with the names of the files ; maybe this was an error and I should have kept a bit of both? And I just stuck standard in a few places because of your comment on the many orderings of ℤ but we can drop them if you prefer.

I think your choice is very sensible, and I can't see why it is a mistake. It seemed rather arbitrary how the prose kept switching between the two terms before.

@fredrik-bakke
Copy link
Collaborator

I like the current version of things, so I think this PR should be merged after my final suggestions have been implemented

@malarbol
Copy link
Contributor Author

It seemed rather arbitrary how the prose kept switching between the two terms before.

well, each contributor will have its own prose and preferred terms so I guess this is bound to happen somehow. But it does give a kind of "organic" touch. As a reader, it's nice to feel that this was written by humans and that it's not just a juxtaposition of automatically derived proofs and generated text.

I like the current version of things, so I think this PR should be merged after my final suggestions have been implemented

thx. I also quite liked how it all turned out. Thank you again for all your time and dedication.

@fredrik-bakke
Copy link
Collaborator

Thank you!

If you don't mind, may I keep myself as co-author of this contribution? Reviewers are automatically added as co-authors when you accept one of their suggestions, but we have a policy that we do not preserve co-authorship status in these cases unless the submitter agrees to it. Normally I wouldn't even ask, but given that we've worked so closely on this one it seems fitting to keep my name on it. I do not mean to take any credit away from you though, you definitely deserve most of it for your hard work!

@malarbol
Copy link
Contributor Author

If you don't mind, may I keep myself as co-author of this contribution?

Of course! I wouldn't have it any other way. I may have written most of the new code but I definitely consider we worked together on this. (and this goes for any future collaborations; I trust your judgment on your level of contribution).

@fredrik-bakke fredrik-bakke merged commit 2cda020 into UniMath:master Mar 28, 2024
4 checks passed
@fredrik-bakke
Copy link
Collaborator

Merged!

@malarbol malarbol deleted the signed-integers branch March 28, 2024 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Small refactorings for elementary number theory
2 participants