#2519 closed enhancement (fixed)
[with patch; positive review] Add support for posets, semi-lattices, etc. to Sage
Reported by: | saliola | Owned by: | saliola |
---|---|---|---|
Priority: | major | Milestone: | sage-3.0.2 |
Component: | combinatorics | Keywords: | |
Cc: | sage-combinat | Merged in: | |
Authors: | Reviewers: | ||
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
Attachments (2)
Change History (12)
Changed 6 years ago by saliola
comment:1 Changed 6 years ago by saliola
- Summary changed from Add support for posets, semi-lattices, etc. to Sage to [with patch; needs criticism] Add support for posets, semi-lattices, etc. to Sage
See announcement and discussion on sage-devel:
http://groups.google.com/group/sage-devel/browse_thread/thread/b6fcc3a134abe22c
comment:2 Changed 6 years ago by mabshoff
- Milestone changed from sage-feature to sage-3.0.1
- Summary changed from [with patch; needs criticism] Add support for posets, semi-lattices, etc. to Sage to [with patch; needs review] Add support for posets, semi-lattices, etc. to Sage
comment:3 follow-up: ↓ 4 Changed 6 years ago by rlm
- Summary changed from [with patch; needs review] Add support for posets, semi-lattices, etc. to Sage to [with patch; some suggestions] Add support for posets, semi-lattices, etc. to Sage
Some comments:
- Lattice should be LatticePoset or something similar. Same comment for Chain, Antichain, Pentagon and Diamond.
- This is a great example of good code. The docstrings are generous and everything is well tested. Nice work!
comment:4 in reply to: ↑ 3 Changed 6 years ago by saliola
- Summary changed from [with patch; some suggestions] Add support for posets, semi-lattices, etc. to Sage to [with patch; needs review] Add support for posets, semi-lattices, etc. to Sage
- Lattice should be LatticePoset or something similar. Same comment for Chain, Antichain, Pentagon and Diamond.
Done. As a remark: we eventually want to create a poset database similar to the graph database. So these poset examples will be moved into the database.
- This is a great example of good code. The docstrings are generous and everything is well tested. Nice work!
Thank you!
There are a few other differences between the original patch and the new patch: 1. Added "or equal to" to the docstrings for is_gequal and is_lequal. 2. I redefined the show methods for FinitePoset and HasseDiagram. Before it was just show = plot, but I since realized this is wrong: show should show the Graphics object returned by the plot method. 3. Added the methods interval, closed_interval and open_interval to the FinitePoset class. These already existed for the HasseDiagram class; so the new methods just expose these methods from the FinitePoset class. It was an oversight not to have done this originally. 4. I slightly modified HasseDiagram.antichains to return the empty list as an antichain. And added tests.
comment:5 follow-up: ↓ 6 Changed 6 years ago by rlm
Regarding show versus plot, you don't even really need a show: most Sage objects give LaTeX when you call show, and if you do sage: P.plot(), the graphics object will appear.
Also, it looks like you need both patches in order, right?
comment:6 in reply to: ↑ 5 Changed 6 years ago by saliola
Replying to rlm:
Regarding show versus plot, you don't even really need a show: most Sage objects give LaTeX when you call show, and if you do sage: P.plot(), the graphics object will appear.
I'll leave it as is, as this is how it is implemented for Graphs/DiGraphs?.
Also, it looks like you need both patches in order, right?
Yes, I patched against the previous patch. I've corrected what I wrote.
I can provide a single patch instead if that is easier.
comment:7 Changed 6 years ago by rlm
- Summary changed from [with patch; needs review] Add support for posets, semi-lattices, etc. to Sage to [with patch; (conditional) positive review] Add support for posets, semi-lattices, etc. to Sage
As long as it applies cleanly and passes all tests...
comment:8 Changed 6 years ago by mabshoff
- Milestone changed from sage-3.0.3 to sage-3.0.2
- Summary changed from [with patch; (conditional) positive review] Add support for posets, semi-lattices, etc. to Sage to [with patch; positive review] Add support for posets, semi-lattices, etc. to Sage
With both patches applied the doctests pass. Two comments:
- sage/combinat/posets/init.py is zero bytes, so the patch doesn't create it. I fixed that manually
- these patches are regular diffs and not mercurial patches, so I ended up with the formal credit when applying them. I did however state in the changelog:
changeset: 9746:0f14afe7ba00 tag: tip user: mabshoff@sage.math.washington.edu date: Thu May 22 16:52:19 2008 -0700 summary: Apply patch by Peter Jipsen and Franco Saliola: Add support for posets, semi-lattices, etc changeset: 9745:0716b13a9e12 user: mabshoff@sage.math.washington.edu date: Thu May 22 16:52:08 2008 -0700 summary: Apply patch by Peter Jipsen and Franco Saliola: Add support for posets, semi-lattices, etc.
Cheers,
Michael
comment:9 Changed 6 years ago by mabshoff
- Resolution set to fixed
- Status changed from new to closed
Merged in Sage 3.0.2.rc0
comment:10 Changed 5 years ago by nthiery
- Cc sage-combinat added; jipsen@… removed
Initial support for finite posets. (Patch against sage-3.0.)