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

LatticePoset: Add certificate for is_join_semidistributive #22137

Closed
jm58660 mannequin opened this issue Jan 4, 2017 · 10 comments
Closed

LatticePoset: Add certificate for is_join_semidistributive #22137

jm58660 mannequin opened this issue Jan 4, 2017 · 10 comments
Labels
c: combinatorics p: minor / 4 t: enhancement
Milestone
sage-7.6

Comments

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Jan 4, 2017

Trivial dual of #22133.

CC: @tscrim

Component: combinatorics

Author: Jori Mäntysalo

Branch/Commit: eac575f

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/22137
@jm58660 jm58660 mannequin added this to the sage-7.6 milestone Jan 4, 2017
@jm58660 jm58660 mannequin added c: combinatorics labels Jan 4, 2017
@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Jan 4, 2017

Branch: u/jmantysalo/join-semidist-cert

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Jan 4, 2017

Commit: 97a2de2

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Jan 4, 2017

New commits:

97a2de2Add certificate to is_join_semidistributive.

@jm58660 jm58660 mannequin added the s: needs review label Jan 4, 2017
@tscrim
Copy link
Collaborator

tscrim commented Jan 4, 2017

comment:3

It would be a little cleaner if you put both if statements in the for loop in one line. If you put the inner one second, short-circuiting will mean it has the same running speed. We should also do this on #22133 too.

@jm58660 jm58660 mannequin added s: needs work and removed s: needs review labels Jan 5, 2017
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 5, 2017

Branch pushed to git repo; I updated commit sha1. New commits:

eac575fFrom if-if to if-and.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 5, 2017

Changed commit from 97a2de2 to eac575f

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Jan 5, 2017

comment:6

Replying to @tscrim:

It would be a little cleaner if you put both if statements in the for loop in one line.

Good point. Done.

@jm58660 jm58660 mannequin added s: needs review and removed s: needs work labels Jan 5, 2017
@tscrim
Copy link
Collaborator

tscrim commented Jan 5, 2017

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Jan 5, 2017

comment:7

LGTM. Thanks.

@vbraun
Copy link
Member

vbraun commented Jan 28, 2017

Changed branch from u/jmantysalo/join-semidist-cert to eac575f

@vbraun vbraun closed this as completed in c1247cb Jan 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c: combinatorics p: minor / 4 t: enhancement
Projects
None yet
Milestone
sage-7.6
Development

No branches or pull requests
2 participants
@vbraun @tscrim