#26854 closed enhancement (fixed)

Test for certificates in finite posets

Reported by: jmantysalo Owned by:
Priority: major Milestone: sage-8.6
Component: combinatorics Keywords:
Cc: chapoton Merged in:
Authors: Jori Mäntysalo Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 9c5fe51 (Commits) Commit: 9c5fe515a52924b77a18b5ea6dc218f0af9a0037
Dependencies: #26861, #26857, #26847 Stopgaps:

Description

An error reported in #26847 shows that tests in src/sage/tests/finite_poset.py do not check that .some_function(certificate=True) returns a pair and .some_function(certificate=True)[0] is always equal to .some_function()

Change History (14)

comment:1 Changed 18 months ago by jmantysalo

  • Cc chapoton added

How to do this automatically and be py3-compatible? With

'certificate' in inspect.getargspec(LatticePoset().is_subdirectly_reducible).args

I can see that there is a certificate-option, but "Python 3.5+ recommends inspect.signature()."

comment:2 follow-up: Changed 18 months ago by chapoton

sage: from sage.misc.sageinspect import sage_getargspec
sage:  sage_getargspec(LatticePoset().is_subdirectly_reducible).args
['self', 'certificate']

comment:3 in reply to: ↑ 2 Changed 18 months ago by jmantysalo

Replying to chapoton:

Thanks, that was fast. So the code can be something like

from sage.misc.sageinspect import sage_getargspec
props = ['is_distributive', 'is_self_dual', 'is_modular']
L = posets.RandomLattice(10, 0.99)
for p in props:
    f = attrcall(p)
    if 'certificate' in sage_getargspec(getattr(L, p)).args:
        if attrcall(p)(L) != attrcall(p, certificate=True)(L)[0]:
            print("Oh no!")

comment:4 Changed 18 months ago by jmantysalo

As an example of bugs to be found with this: #26857.

Nice way to handle is_doubling_constructible and other currently handled by test_attrcall is needed.

comment:5 Changed 18 months ago by jmantysalo

  • Branch set to u/jmantysalo/test_for_certificates_in_finite_posets

comment:6 Changed 18 months ago by jmantysalo

  • Authors set to Jori Mäntysalo
  • Commit set to cfb20e029808fdb76371f5ad8b26ad7933c4667a

Untested code for lattices.

More to be done for general posets.


New commits:

cfb20e0Test results with certificate=True

comment:7 Changed 18 months ago by git

  • Commit changed from cfb20e029808fdb76371f5ad8b26ad7933c4667a to 9c5fe515a52924b77a18b5ea6dc218f0af9a0037

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

9c5fe51Test also finite non-lattice posets.

comment:8 Changed 18 months ago by jmantysalo

  • Dependencies set to #26861, #26857, #26847
  • Status changed from new to needs_review

comment:9 Changed 17 months ago by jmantysalo

This should now be ready to review, as all dependencies have been solved.

comment:10 Changed 17 months ago by tscrim

  • Milestone changed from sage-8.5 to sage-8.6
  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

LGTM.

comment:11 Changed 17 months ago by mantepse

  • Status changed from positive_review to needs_work

I think pyflakes spotted an error:

+src/sage/tests/finite_poset.py:16: dictionary key 'doubling_interval' repeated with different values
+src/sage/tests/finite_poset.py:17: dictionary key 'doubling_interval' repeated with different values

comment:12 Changed 17 months ago by tscrim

  • Status changed from needs_work to positive_review

That is both not in these changes and not a reason to block a positive review.

comment:13 Changed 17 months ago by mantepse

sorry.

comment:14 Changed 17 months ago by vbraun

  • Branch changed from u/jmantysalo/test_for_certificates_in_finite_posets to 9c5fe515a52924b77a18b5ea6dc218f0af9a0037
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.