Opened 3 years ago

Closed 3 years ago

#21505 closed enhancement (fixed)

LatticePoset: certificate for is_pseudocomplemented()

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

Description

Add an option to get an element without the pseudocomplement. (Compare to .is_complemented(certificate=True).

Change History (13)

comment:1 Changed 3 years ago by jmantysalo

  • Branch set to u/jmantysalo/pseudocomp-certificate

comment:2 Changed 3 years ago by jmantysalo

  • Commit set to 4785ba6b14177a4eb02156c0246b6c6c7e5f6975
  • Status changed from new to needs_review

Should it be "an element without the pseudocomplement" or "an element without a pseudocomplement", as an element can have at most one pseudocomplemet?


New commits:

4785ba6Add certificate-option to is_pseudocomplemented().

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

I would say "a pseudocomplement" both in case one does not exist and so it makes it seem less like there is one pseudocomplement for all lattices (which you could get around by saying "the pseudocomplement of the lattice").

comment:4 Changed 3 years ago by git

  • Commit changed from 4785ba6b14177a4eb02156c0246b6c6c7e5f6975 to 1ed0863085418ba587f7e63bacab94bf9a5d2e65

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

1ed0863'the' or 'a'.

comment:5 in reply to: ↑ 3 Changed 3 years ago by jmantysalo

Replying to tscrim:

I would say "a pseudocomplement" - -

OK. Thanks.

comment:6 Changed 3 years ago by jmantysalo

Travis, can you also review this? Should be trivial.

(Not that important, but kind of nice to have unified format of is_* -functions when possbile.)

Last edited 3 years ago by jmantysalo (previous) (diff)

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

  • Reviewers set to Travis Scrimshaw

Is there a reason why you have one as 3 lines if-return and the other as a 1 line return foo if bar else True? I think it would be good to be consistent. Once addressed, you can set a positive review on my behalf.

comment:8 Changed 3 years ago by git

  • Commit changed from 1ed0863085418ba587f7e63bacab94bf9a5d2e65 to 69e0b1eef3cca6283df4a3e485642522a15202b6

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

69e0b1eReformatting return-if-else.

comment:9 in reply to: ↑ 7 Changed 3 years ago by jmantysalo

  • Status changed from needs_review to positive_review

Replying to tscrim:

Is there a reason why you have one as 3 lines if-return and the other as a 1 line return foo if bar else True? I think it would be good to be consistent. Once addressed, you can set a positive review on my behalf.

Done that. Thanks.

comment:10 follow-up: Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work
sage -t --long src/sage/combinat/posets/lattices.py
**********************************************************************
File "src/sage/combinat/posets/lattices.py", line 1272, in sage.combinat.posets.lattices.FiniteLatticePoset.is_pseudocomplemented
Failed example:
    L.is_pseudocomplemented(certificate=True)
Exception raised:
    Traceback (most recent call last):
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 498, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 861, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.combinat.posets.lattices.FiniteLatticePoset.is_pseudocomplemented[4]>", line 1, in <module>
        L.is_pseudocomplemented(certificate=True)
    TypeError: is_pseudocomplemented() got an unexpected keyword argument 'certificate'
**********************************************************************
1 item had failures:
   1 of   7 in sage.combinat.posets.lattices.FiniteLatticePoset.is_pseudocomplemented
    [347 tests, 1 failure, 1.10 s]

comment:11 in reply to: ↑ 10 Changed 3 years ago by jmantysalo

Replying to vbraun:

        L.is_pseudocomplemented(certificate=True)
    TypeError: is_pseudocomplemented() got an unexpected keyword argument 'certificate'

??? I can not get that error message. It seems that the code has been tested before compiling.

comment:12 Changed 3 years ago by tscrim

  • Status changed from needs_work to positive_review

I agree with Jori's assessment.

comment:13 Changed 3 years ago by vbraun

  • Branch changed from u/jmantysalo/pseudocomp-certificate to 69e0b1eef3cca6283df4a3e485642522a15202b6
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.