#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: | dbe5cbd894e2771f3f430ec310120af68b06022a
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
> }}}
>
> 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 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
--
Comment (by ncohen):
Oh. I see. That's for
{{{
sage: Words("abcd") == WordPaths("abcd")
True
}}}
Then for this case I will keep the old behaviour.
Ready for review ! This is only hacks, but it fixes the wrong results
`:-/`
Nathann
----
New commits:
||[[http://git.sagemath.org/sage.git/commit/?id=dbe5cbd|dbe5cbd]]||trac
#15480: Words.__eq__ returns wrong answers||
----
New commits:
||[[http://git.sagemath.org/sage.git/commit/?id=dbe5cbd|dbe5cbd]]||trac
#15480: Words.__eq__ returns wrong answers||
--
Ticket URL: <http://trac.sagemath.org/ticket/15480#comment:3>
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.
