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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - feat(data/sym): Provide API for data.sym #11032

Closed
wants to merge 25 commits into from
Closed

[Merged by Bors] - feat(data/sym): Provide API for data.sym #11032

wants to merge 25 commits into from

Conversation

huynhtrankhanh
Copy link
Collaborator

Open in Gitpod

jcommelin
jcommelin reviewed Dec 24, 2021
View reviewed changes
src/data/multiset/basic.lean Outdated Show resolved Hide resolved
src/data/sym/basic.lean Outdated Show resolved Hide resolved
src/data/sym/basic.lean Outdated Show resolved Hide resolved
src/data/sym/basic.lean Outdated Show resolved Hide resolved
src/data/sym/basic.lean Outdated Show resolved Hide resolved
src/data/sym/basic.lean Outdated Show resolved Hide resolved
src/data/sym/sym2.lean Outdated Show resolved Hide resolved
src/data/sym/sym2.lean Outdated Show resolved Hide resolved
src/data/sym/sym2.lean Outdated Show resolved Hide resolved
src/data/sym/sym2.lean Outdated Show resolved Hide resolved
adomani
adomani reviewed Dec 24, 2021
View reviewed changes
src/data/sym/basic.lean Outdated Show resolved Hide resolved
@eric-wieser eric-wieser added the awaiting-author A reviewer has asked the author a question or requested changes label Dec 24, 2021
@huynhtrankhanh huynhtrankhanh added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Dec 25, 2021
jcommelin
jcommelin reviewed Dec 25, 2021
View reviewed changes
src/data/multiset/basic.lean Outdated Show resolved Hide resolved
src/data/sym/basic.lean Outdated Show resolved Hide resolved
src/data/sym/basic.lean Show resolved Hide resolved
src/data/sym/basic.lean Show resolved Hide resolved
src/data/sym/basic.lean Outdated Show resolved Hide resolved
@jcommelin jcommelin added awaiting-author A reviewer has asked the author a question or requested changes needs-documentation This PR is missing required documentation and removed awaiting-review The author would like community review of the PR labels Dec 25, 2021
@huynhtrankhanh huynhtrankhanh removed needs-documentation This PR is missing required documentation awaiting-author A reviewer has asked the author a question or requested changes labels Dec 25, 2021
@huynhtrankhanh huynhtrankhanh added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Dec 25, 2021
@huynhtrankhanh huynhtrankhanh added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Dec 26, 2021
kmill
kmill approved these changes Dec 26, 2021
View reviewed changes
Copy link
Collaborator

@kmill kmill left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this!

The only thing I'm not sure about is the has_zero and has_emptyc instances. has_zero makes some sense, but there's not a monoid structure on sym -- maybe it would be better to remove these and write nil explicitly instead of 0? (I'm not telling you to remove them, but I just wanted to point it out for others to weigh in.)

kmill
kmill reviewed Dec 26, 2021
View reviewed changes
src/data/sym/basic.lean Outdated Show resolved Hide resolved
urkud
urkud reviewed Dec 26, 2021
View reviewed changes
src/data/multiset/basic.lean Outdated Show resolved Hide resolved
@urkud
Copy link
Member

urkud commented Dec 27, 2021

I golfed some proofs.

jcommelin
jcommelin approved these changes Dec 27, 2021
View reviewed changes
Copy link
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

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

Thanks 🎉

bors merge

bors bot pushed a commit that referenced this pull request Dec 27, 2021
@huynhtrankhanh @urkud
feat(data/sym): Provide API for data.sym (#11032)
b28734c 


Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
@github-actions github-actions bot added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Dec 27, 2021
@bors
Copy link

bors bot commented Dec 27, 2021

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(data/sym): Provide API for data.sym [Merged by Bors] - feat(data/sym): Provide API for data.sym Dec 27, 2021
@bors bors bot closed this Dec 27, 2021
@bors bors bot deleted the huynhtrankhanh/api-for-sym branch December 27, 2021 13:56
erdOne pushed a commit that referenced this pull request Jan 1, 2022
@huynhtrankhanh @urkud @erdOne
feat(data/sym): Provide API for data.sym (#11032)
f4fec63 


Co-authored-by: Yury G. Kudryashov <urkud@urkud.name>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@urkud urkud urkud left review comments

@kmill kmill kmill approved these changes

@eric-wieser eric-wieser eric-wieser left review comments

@YaelDillies YaelDillies YaelDillies left review comments

@adomani adomani adomani left review comments

@jcommelin jcommelin jcommelin approved these changes

Assignees
No one assigned
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@huynhtrankhanh @urkud @kmill @eric-wieser @jcommelin @YaelDillies @adomani