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: sage-7.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:

Description

Check documentation for product(), dual() and so on.

This continues the serie of #18925, #18941, #18959, #19141 and #19360.

Change History (11)

comment:1 Changed 5 years ago by jmantysalo

  • Branch set to u/jmantysalo/poset_documentation_polishing__new_posets_from_old_ones

comment:2 Changed 5 years ago by jmantysalo

  • Commit set to 41e3a4b15d9e38f74cf865afd04a3b79d1fa38ce
  • Milestone changed from sage-6.10 to sage-7.2
  • Status changed from new to needs_review

This is a long but quite trivial patch. Mostly bikeshedding.

I don't know easy way to make with_bounds() work with non-facade posets. At least now the code gives nicer error message. I also changed completion_by_cuts() to return the empty lattice from the empty poset. It is now consistent with definition "smallest lattice containing..."


New commits:

41e3a4bSlight modifications to documentation.

comment:3 follow-ups: Changed 5 years ago by kdilks

  • Potential issue with the example in connected_components() giving the covering relations of the first connected component, since cover_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 Dedekind-Macneille 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 jmantysalo

Replying to kdilks:

  • Potential issue with the example in connected_components() giving the covering relations of the first connected component, since cover_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 jmantysalo

  • Status changed from needs_review to needs_work

Replying to kdilks:

  • I'm not happy about the definition of Dedekind-Macneille 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 git

  • Commit changed from 41e3a4b15d9e38f74cf865afd04a3b79d1fa38ce to 4283464dca5b8205bc699a2864ef5182b296918b

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

4283464Some fixes to poset documentation.

comment:7 Changed 4 years ago by git

  • Commit changed from 4283464dca5b8205bc699a2864ef5182b296918b to e3e3aa988957963555b8ae912f1b5f5dc1620349

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

f8956b9Slight modifications to documentation.
f436b4bSome fixes to poset documentation.
e3e3aa9Rebase and minor tweaks.

comment:8 Changed 4 years ago by jmantysalo

  • Status changed from needs_work to needs_review

Continuing with this again. Nothing special in this.

comment:9 Changed 4 years ago by kdilks

  • Reviewers set to Kevin Dilks
  • Status changed from needs_review to positive_review

comment:10 Changed 4 years ago by jmantysalo

  • Milestone changed from sage-7.2 to sage-7.4

Thanks Kevin. rc0 is out, so I changed milestone.

comment:11 Changed 4 years ago by vbraun

  • 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
Note: See TracTickets for help on using tickets.