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

Subset_s _an_element_ #33990

Closed
trevorkarn opened this issue Jun 13, 2022 · 9 comments
Closed

Subset_s _an_element_ #33990

trevorkarn opened this issue Jun 13, 2022 · 9 comments

Comments

@trevorkarn
Copy link
Contributor

Currently the Subset_s.an_element() method returns a different result from the Subset_s._an_element_() method. The proposed fix is to move the current implementation of an_element to _an_element_ and let the parent framework take care of the rest.

sage: from sage.combinat.subset import Subsets_s
sage: Subsets_s([1,2,3])._an_element_()
{}
sage: Subsets_s([1,2,3]).an_element()
{1, 2}

CC: @tscrim

Component: combinatorics

Keywords: gsoc2022 subsets elements

Author: Trevor K. Karn

Branch/Commit: 922c40a

Reviewer: Travis Scrimshaw

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

@trevorkarn trevorkarn added this to the sage-9.7 milestone Jun 13, 2022
@trevorkarn

This comment has been minimized.

@trevorkarn
Copy link
Contributor Author

New commits:

922c40aChange an_element to _an_element_

@trevorkarn
Copy link
Contributor Author

Commit: 922c40a

@trevorkarn
Copy link
Contributor Author

Branch: u/tkarn/33990-an_element

@tscrim
Copy link
Collaborator

tscrim commented Jun 14, 2022

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Jun 14, 2022

comment:4

Strictly speaking, this is not a bug: we provide a default implementation of an_element() that caches the result of _an_element_() (which has its own generic implementation). However, we have no requirement that they both need to be implemented non-generically and return the same result. However, I do find that by using the caching mechanism this becomes an improvement. Once the patchbot comes around and is green, then this is a positive review.

@trevorkarn
Copy link
Contributor Author

comment:5

It looks like a plugin failed, but I am not sure how to read the message. Do you have any advice?

@tscrim
Copy link
Collaborator

tscrim commented Jun 14, 2022

comment:6

That one can be ignored.

@vbraun
Copy link
Member

vbraun commented Jun 21, 2022

Changed branch from u/tkarn/33990-an_element to 922c40a

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

3 participants