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

Allow with_bounds to work on non-facade posets #25846

Closed
deinst opened this issue Jul 12, 2018 · 10 comments
Closed

Allow with_bounds to work on non-facade posets #25846

deinst opened this issue Jul 12, 2018 · 10 comments

Comments

@deinst
Copy link
Contributor

deinst commented Jul 12, 2018

Currently with_bounds is not implemented on non-facade posets. This may or not be a good idea, as often one would want the elements in the old poset and the poset with bounds to be comparable, and therefore having facade=True, but the current behavior is surprising.

We should either document that with_bounds does not work with non-facade posets, or create an implementation

Component: combinatorics

Author: David Einstein

Branch/Commit: 3658311

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/25846

@deinst deinst added this to the sage-8.4 milestone Jul 12, 2018
@deinst
Copy link
Contributor Author

deinst commented Jul 12, 2018

Branch: u/deinst/fix_poset_with_bounds

@deinst
Copy link
Contributor Author

deinst commented Jul 12, 2018

Commit: d061ea0

@deinst
Copy link
Contributor Author

deinst commented Jul 12, 2018

New commits:

d061ea0Allow with_bounds to work on non-facade posets

@tscrim
Copy link
Collaborator

tscrim commented Jul 12, 2018

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Jul 12, 2018

comment:3

Trivial PEP8 thing:

-P = posets.PentagonPoset(facade = False)
+P = posets.PentagonPoset(facade=False)

Once changed, you can set a positive review on my behalf.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 12, 2018

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

3658311Fixed extra spaces in named parameter.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 12, 2018

Changed commit from d061ea0 to 3658311

@deinst
Copy link
Contributor Author

deinst commented Jul 12, 2018

comment:5

Sorry, Thanks.

@fchapoton
Copy link
Contributor

Changed author from David EInstein to David Einstein

@vbraun
Copy link
Member

vbraun commented Aug 29, 2018

Changed branch from u/deinst/fix_poset_with_bounds to 3658311

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants