Opened 5 years ago
Closed 4 years ago
#19435 closed enhancement (fixed)
Poset documentation polishing: New posets from old ones
Reported by:  jmantysalo  Owned by:  

Priority:  minor  Milestone:  sage7.4 
Component:  documentation  Keywords:  
Cc:  Merged in:  
Authors:  Jori Mäntysalo  Reviewers:  Kevin Dilks 
Report Upstream:  N/A  Work issues:  
Branch:  e3e3aa9 (Commits)  Commit:  e3e3aa988957963555b8ae912f1b5f5dc1620349 
Dependencies:  Stopgaps: 
Change History (11)
comment:1 Changed 5 years ago by
 Branch set to u/jmantysalo/poset_documentation_polishing__new_posets_from_old_ones
comment:2 Changed 5 years ago by
 Commit set to 41e3a4b15d9e38f74cf865afd04a3b79d1fa38ce
 Milestone changed from sage6.10 to sage7.2
 Status changed from new to needs_review
comment:3 followups: ↓ 4 ↓ 5 Changed 5 years ago by
 Potential issue with the example in
connected_components()
giving the covering relations of the first connected component, sincecover_relations()
returns an ordered list, but strictly speaking is an unordered set. Not really an issue, since I imagine the code will always return it in the order given, but it's something to think about.
 When talking about dual/ordinal sum of a lattice (resp join/meet semilattices), the documentation appears to use 'lattice' etc. as adjectives (ie, 'the dual of a lattice is lattice'). All of those should be like 'the dual of a lattice is a lattice'.
 I'm not happy about the definition of DedekindMacneille completion being vague about the concept of 'smallest' and not being explicit about the original poset being an induced subposet of the lattice, but I suppose it matches what Wikipedia has, and does link to the full Wikipedia article.
comment:4 in reply to: ↑ 3 Changed 5 years ago by
Replying to kdilks:
 Potential issue with the example in
connected_components()
giving the covering relations of the first connected component, sincecover_relations()
returns an ordered list, but strictly speaking is an unordered set. Not really an issue, since I imagine the code will always return it in the order given, but it's something to think about.
This is true, and I do not know a good way to overcome this. # random order
is one possibility. For a user perspective I see no problem when documentation says that a function returns [3, 5]
, but he/she gots [5, 3]
; if the user has any clue at all, he will understand that the list represents a set.
For some posets we could use linear_extension=True
, but it only applies to list of elements, and is kind of noise to the user.
comment:5 in reply to: ↑ 3 Changed 5 years ago by
 Status changed from needs_review to needs_work
Replying to kdilks:
 I'm not happy about the definition of DedekindMacneille completion being vague about the concept of 'smallest' and not being explicit about the original poset being an induced subposet of the lattice, but I suppose it matches what Wikipedia has, and does link to the full Wikipedia article.
Could we have an option to show this clearly? I.e. somehow show what are "original" and "added" elements on the completion?
comment:6 Changed 5 years ago by
 Commit changed from 41e3a4b15d9e38f74cf865afd04a3b79d1fa38ce to 4283464dca5b8205bc699a2864ef5182b296918b
Branch pushed to git repo; I updated commit sha1. New commits:
4283464  Some fixes to poset documentation.

comment:7 Changed 4 years ago by
 Commit changed from 4283464dca5b8205bc699a2864ef5182b296918b to e3e3aa988957963555b8ae912f1b5f5dc1620349
comment:8 Changed 4 years ago by
 Status changed from needs_work to needs_review
Continuing with this again. Nothing special in this.
comment:9 Changed 4 years ago by
 Reviewers set to Kevin Dilks
 Status changed from needs_review to positive_review
comment:10 Changed 4 years ago by
 Milestone changed from sage7.2 to sage7.4
Thanks Kevin. rc0 is out, so I changed milestone.
comment:11 Changed 4 years ago by
 Branch changed from u/jmantysalo/poset_documentation_polishing__new_posets_from_old_ones to e3e3aa988957963555b8ae912f1b5f5dc1620349
 Resolution set to fixed
 Status changed from positive_review to closed
This is a long but quite trivial patch. Mostly bikeshedding.
I don't know easy way to make
with_bounds()
work with nonfacade posets. At least now the code gives nicer error message. I also changedcompletion_by_cuts()
to return the empty lattice from the empty poset. It is now consistent with definition "smallest lattice containing..."New commits:
Slight modifications to documentation.