Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#19190 closed enhancement (fixed)

LatticePoset: add atoms, coatoms, doubly irreducibles etc.

Reported by: jmantysalo Owned by:
Priority: minor Milestone: sage-7.2
Component: combinatorics Keywords: latticeposet
Cc: Merged in:
Authors: Jori Mäntysalo Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: aa58591 (Commits) Commit:
Dependencies: Stopgaps:

Description (last modified by jmantysalo)

Add atoms(), coatoms(), and doubly_irreducibles() to finite lattices.

Change History (13)

comment:1 Changed 4 years ago by jmantysalo

  • Description modified (diff)

comment:2 Changed 4 years ago by jmantysalo

  • Description modified (diff)
  • Summary changed from LatticePoset: add atoms, coatoms, doubly irreducibles to LatticePoset: add atoms, coatoms, doubly irreducibles etc.

comment:3 Changed 4 years ago by jmantysalo

  • Branch set to u/jmantysalo/latticeposet__add_atoms__coatoms__doubly_irreducibles_etc_

comment:4 Changed 4 years ago by jmantysalo

  • Commit set to 429b494b5f553952bf6ed88fe1f1d9dc744d2f15
  • Description modified (diff)
  • Keywords latticeposet added; poset removed
  • Milestone changed from sage-wishlist to sage-7.2
  • Status changed from new to needs_review

This patch will add three functions. Also this will make LatticePoset() to return the empty lattice; compare to Poset().


New commits:

429b494Add atoms() etc.

comment:5 Changed 4 years ago by tscrim

On my to-review list.

comment:6 Changed 3 years ago by tscrim

  • Branch changed from u/jmantysalo/latticeposet__add_atoms__coatoms__doubly_irreducibles_etc_ to public/posets/add_methods-19190
  • Commit changed from 429b494b5f553952bf6ed88fe1f1d9dc744d2f15 to aa58591d18e75fd32ae3465a710c1d270e4d9e1f
  • Reviewers set to Travis Scrimshaw

I did some (additional) touchups to the finite lattice category and minor tweaks. While I still prefer ``self`` over this lattice, the lattice is not correct as there is not a unique lattice. If you agree with my changes, then go ahead and set a positive review.


New commits:

376adfbSome cleanup in the category docstrings.
aa58591Some touchups of lattices.py.

comment:7 Changed 3 years ago by jmantysalo

In html documentation "See also: FinitePosets?, LatticePosets?, LatticePoset?" the "LatticePoset?" is a broken link. Where is it supposed to point?

About "the": I don't understand. For example docstring for cardinality() is "Return the number of elements in the poset." I have think that "the" means about same as "this" in that sentence.

comment:8 Changed 3 years ago by jmantysalo

Btw, I run doctests and they were successfull. So patchbots again give false errors.

comment:9 Changed 3 years ago by jmantysalo

  • Status changed from needs_review to positive_review

Anyways, Sage with this patch is better than without, hence I mark this as positive_review.

Broken links are more general problem. Nathann give one suggestion at #20095, but that should be talked separately.

comment:10 Changed 3 years ago by vbraun

  • Branch changed from public/posets/add_methods-19190 to aa58591d18e75fd32ae3465a710c1d270e4d9e1f
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:11 follow-up: Changed 3 years ago by tscrim

  • Commit aa58591d18e75fd32ae3465a710c1d270e4d9e1f deleted

"the" implies uniqueness, but there is not a unique poset. "this" within the context gives uniqueness. In fact, I would change all of those "the" to "this".

comment:12 in reply to: ↑ 11 ; follow-up: Changed 3 years ago by jmantysalo

Replying to tscrim:

"the" implies uniqueness, but there is not a unique poset. "this" within the context gives uniqueness. In fact, I would change all of those "the" to "this".

Trying to understand... There is no direct translation for a/an/the in Finnish.

I have learnt that if I say "Travis, open the window", it means that I have one specific window in my mind and I suppose that you also know what window I mean. "Open a window" means that there are several to choose one, and I don't care which one you open.

In "Return the number of elements in the poset." I suppose that it is clear what poset we are referring to.

comment:13 in reply to: ↑ 12 Changed 3 years ago by tscrim

Replying to jmantysalo:

Replying to tscrim:

"the" implies uniqueness, but there is not a unique poset. "this" within the context gives uniqueness. In fact, I would change all of those "the" to "this".

Trying to understand... There is no direct translation for a/an/the in Finnish.

I have absolutely zero understanding of Finnish (I don't think I've ever really encountered any before).

I have learnt that if I say "Travis, open the window", it means that I have one specific window in my mind and I suppose that you also know what window I mean. "Open a window" means that there are several to choose one, and I don't care which one you open.

Yes, provided there is only one window we are discussing.

In "Return the number of elements in the poset." I suppose that it is clear what poset we are referring to.

In a way, yes, but it is more precise to say "this poset" as there might be one more poset around (especially when there is another input of a poset).

Note: See TracTickets for help on using tickets.