Posets: with_linear_extension() and wrong constructor

[jmantysalo]

sage: P = Posets.PentagonPoset()
sage: type(P), P.category()
(<class 'sage.combinat.posets.lattices.FiniteLatticePoset_with_category'>,
 Join of Category of finite lattice posets . . .
sage: P_ = P.with_linear_extension([0, 1, 3, 2, 4])
sage: type(P_), P_.category()
(<class 'sage.combinat.posets.posets.FinitePoset_with_category'>,
 Join of Category of finite lattice posets . . .

and so

sage: P.meet_irreducibles(), P_.meet_irreducibles()
([1, 2, 3], [1, 3, 2])

but

sage: P.double_irreducibles()
[1, 2, 3]
sage: P_.double_irreducibles()
AttributeError  Traceback (most recent call last)

[jmantysalo]

The patch contains also slight modifications to non-related docstrings.

---

New commits:

​d86222b

Correction to with_linear_extension + some more.

[tscrim]

This is completely non-future-proof. A better solution would be to use self.__class__ or something to this affect. I'm still -1 on removing ``self`` from docstrings.

[jmantysalo]

CC to Nicolas, as the question is more general.

Replying to tscrim:

This is completely non-future-proof. A better solution would be to use self.__class__ or something to this affect.

Nope. Only really foolproof solution is to have base class A to never know anything about subclasses, only give hooks for them. A would have something like

def _give_f_constructor(self): return A
def f():
    . . . something here, for example, creates X . . .
    constructor = self._give_f_constructor()
    return constructor(X)

and then B might have

def _give_f_constructor(self): return B

It is not possible for a class A to know if somebody will add sub-sub-classes B -> C -> D and so on, so that for example D.f() should return a type of C. Complement of a bipartite graph was an example of this.

But in reality we can't achieve that. So, I can make the construction with __class__. But if the category system gives some solution to this problem, I will hear.

I'm still -1 on removing ``self`` from docstrings.

Yeah, should be resolved in a way or another. Belongs to the same class than "certificate=" vs. "certify=", "algorithm=" vs. "implementation=" etc.

[git]

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

​07f7f86

A non-working example.

[jmantysalo]

Here is an example with __class__. It fails for promotion() and evacuation().

[tscrim]

It is a subtle problem with UniqueRepresentation in that the class itself is part of the key for the cache. So what we have to do is pull out the actual class instead of the one created by the category framework. The category framework guarantees that the original class is in position 1 in the MRO (otherwise, the category methods would override the concrete classes), so this is safe to do.

---

New commits:

​971067d

Get the actual class, not the one created by the category, and some other tweaks.

[jmantysalo]

Replying to tscrim:

It is a subtle problem with UniqueRepresentation in that the class itself is part of the key for the cache. So what we have to do is pull out the actual class instead of the one created by the category framework. The category framework guarantees that the original class is in position 1 in the MRO (otherwise, the category methods would override the concrete classes), so this is safe to do.

OK. Hope that Frédéric or Nicolas can review this, as I don't understand the category system.

Now, for example relabel() contains code block starting

if isinstance(self, FiniteLatticePoset):
    constructor = FiniteLatticePoset
elif isinstance(self, FiniteMeetSemilattice):
    constructor = FiniteMeetSemilattice

Should we change that too?

[jmantysalo]

Now clicking branch shows whole posets.py in red. Is this just a bug of Trac, or is there some real errors, conflicts or something?

[tscrim]

trac bug.

[jmantysalo]

Replying to jmantysalo:

OK. Hope that Frédéric or Nicolas can review this, as I don't understand the category system.

Ping. Travis can not be the only one who knows what self.__class__.__mro__[1] does and if can got broken or not.

[git]

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

​334aab9	Merge branch 'public/posets/with_linear_extension-21607' of trac.sagemath.org:sage into public/posets/with_linear_extension-21607
​9a9126e	Using __base__ instead of __mro__[1].

[jmantysalo]

Travis said that Nicolas has checked the mro-line, and so I mark this as positive review.