#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 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
New description:
Right now equality between sets of words only compares 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):
Replying to [comment:7 slabbe]:
> Personnally, I would delete the file {{{combinat/words/paths.py}}} and
rewrite it all. So if there is some problem with it, don't try too much to
fix it...
Unfortunately because of sage's depreciation policy we can't just remove
{{{paths.py}}}, even if this is the ''right'' thing to do.
I was going to suggest that we instead remove the doc tests of the form
sage: Words("abcd") == WordPaths("abcd")
True
}}}
Without these we could have a more natural looking equality test.
Unfortunately, with this "more natural" equiality test there are now
multiple doctest failures in finite_word.py and in word_generators.py. So
this is not a good solution. I have not drilled down to find out why these
doctests fail because this does not happen with Nathann's patch. So, as
unnatural as the code looks, it is perhaps the best way to go.
Andrew
--
Ticket URL: <http://trac.sagemath.org/ticket/15480#comment:9>
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.
