#15480: Words.__eq__ returns wrong answers
-------------------------+-------------------------------------------------
Reporter: | Owner:
ncohen | Status: needs_review
Type: | Milestone: sage-5.13
defect | Resolution:
Priority: major | Merged in:
Component: | Reviewers:
combinatorics | Work issues:
Keywords: | Commit:
Authors: | ad1cb6306d1dd48b2aa91c92ca00bf10fe1430b2
Nathann Cohen | Stopgaps:
Report Upstream: N/A |
Branch: |
u/ncohen/15480 |
Dependencies: |
-------------------------+-------------------------------------------------
Old description:
> Right now equality between sets of words only compare the alphabets
>
> {{{
> sage: Words(3, 10) == Words(3,900)
> True
> sage: Words(2, finite=False) == Words(2)
> True
> sage: Words(2) == Words(2,30)
> True
> sage: Words(10,0) == Words(20,0)
> False
> sage: WordPaths('abcd') == Words("abcd",3)
> True
> }}}
>
> I am not proud of this patch's code, but I see NO other way to all these
> wrongs answers without rewriting the class hierarchy. The fact that a
> class of finite words is an instance (i.e. it inherits) from a class of
> infinite words makes it really hard to implement `O_o`
>
> {{{
> sage: isinstance(Words(4,30), type(Words(3)))
> True
> }}}
>
> You cannot be sure, when implementing the `__eq__` method of `Words_all`,
> that self really represents an infinite class of words.
>
> Besides, the old code read :
>
> {{{
> if isinstance(other, Words_all):
> return self.alphabet() == other.alphabet()
> else:
> return NotImplemented
> }}}
>
> I haven't been able to guess when `not (type(self) is type(other))` does
> not mean that the two classes should not be reported as equal.
> Though the old code seems to imply that this can happen.
>
> If this can happen we need to add a doctest somewhere.
>
> Nathann
New description:
Right now equality between sets of words only compare the alphabets:
{{{
sage: Words(3, 10) == Words(3,900)
True
sage: Words(2, finite=False) == Words(2)
True
sage: Words(2) == Words(2,30)
True
sage: Words(10,0) == Words(20,0)
False
sage: WordPaths('abcd') == Words("abcd",3)
True
}}}
I am not proud of this patch's code, but I see NO other way to fix all
these incorrect answers without rewriting the class hierarchy. The fact
that a class of finite words is an instance (i.e. it inherits) from a
class of infinite words makes it really hard to implement `O_o`
{{{
sage: isinstance(Words(4,30), type(Words(3)))
True
}}}
You cannot be sure, when implementing the `__eq__` method of `Words_all`,
that self really represents an infinite class of words.
Besides, the old code reads:
{{{
if isinstance(other, Words_all):
return self.alphabet() == other.alphabet()
else:
return NotImplemented
}}}
I haven't been able to guess when `not (type(self) is type(other))` does
not mean that the two classes should not be reported as equal.
Though the old code seems to imply that this can happen.
If this can happen we need to add a doctest somewhere.
Nathann
--
Comment (by andrew.mathas):
I was just looking at this and I agree it's a mess. I thought that you
might be able to get away with just
{{{#!python
if self.cardinality()!=other.cardinality():
return False
if self.cardinality()==1:
return True
if self.alphabet() != other.alphabet():
return False
return type(self)==type(other)
}}}
but this does not catch
{{{
sage: Words("abcd") == WordPaths("abcd")
True
}}}
As I don't really play with these things it is not clear to me that these
two should be equal.
Is this really correct?
--
Ticket URL: <http://trac.sagemath.org/ticket/15480#comment:5>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/groups/opt_out.
