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

Change SetSystem representation #37904

Merged
merged 1 commit into from
May 2, 2024
Merged

Conversation

gmou3
Copy link
Contributor

@gmou3 gmou3 commented Apr 29, 2024

This is an attempt to represent SetSystem in a more informative and user-friendly way.

Previous:

sage: M = matroids.CompleteGraphic(7)
sage: M.bases()
Iterator over a system of subsets

Current:

sage: M = matroids.CompleteGraphic(7)
sage: M.bases()
SetSystem of 16807 sets over 21 elements

I think this is an improvement from resorting to calling, e.g., len, in order to to get some info, after you get struck with Iterator over a system of subsets.
Note that a SetSystem is not an iterator but simply an iterable (so __repr__ is also misleading). The previous developers were planning to make it into an actual iterator, as can be seen on L66 of set_system.pyx.

Copy link

Documentation preview for this PR (built with commit f1dc325; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@tscrim
Copy link
Collaborator

tscrim commented May 1, 2024

This is an improvement given the current implementation. If it does become an actual iterator (or a "view" like Python_dict.keys()), it might know some additional information about its size, At that time, we can revisit the repr output.

@tscrim
Copy link
Collaborator

tscrim commented May 1, 2024

@gmou3 If this is ready for review, just let me know. I am ready to set a positive review if ready.

@gmou3
Copy link
Contributor Author

gmou3 commented May 1, 2024

Yeap, it is ready I think. Thanks for the review.

Copy link
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

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

Thanks.

@vbraun vbraun merged commit 67f2675 into sagemath:develop May 2, 2024
17 checks passed
@mkoeppe mkoeppe added this to the sage-10.4 milestone May 2, 2024
@gmou3 gmou3 deleted the setsystem__repr__ branch May 25, 2024 15:30
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.

4 participants