Opened 4 years ago
Closed 4 years ago
#21980 closed enhancement (fixed)
py3 richcmp in real_double pyx
Reported by: | chapoton | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-7.5 |
Component: | python3 | Keywords: | |
Cc: | tscrim, aapitzsch, jdemeyer | Merged in: | |
Authors: | Frédéric Chapoton | Reviewers: | Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | dbeac6d (Commits) | Commit: | dbeac6d608d4d26db0404ccb1bd9a6a569c2ab17 |
Dependencies: | Stopgaps: |
Description
a minuscule step to python3
Change History (15)
comment:1 Changed 4 years ago by
- Branch set to u/chapoton/21980
- Commit set to 5eb675f404462f5af4225f223838638e2c1595e7
comment:2 Changed 4 years ago by
_richcmp_
only makes sense for Element
classes. Do we have a general strategy for Parent
classes?
comment:3 Changed 4 years ago by
I would say parents should only compare equal if they are the same (in which sense ?).
Converting to __richcmp__
seems to be causing problems with pickling.
I have currently no idea what to do, but this is standing on our long way to py3.
comment:4 Changed 4 years ago by
Perhaps change it to an __eq__
and add a __ne__
?
comment:5 Changed 4 years ago by
Actually, I think what was causing the problems with simply changing to __richcmp__
is with the blanket return NotImplemented
. This was not actually checking !=
.
comment:6 Changed 4 years ago by
- Commit changed from 5eb675f404462f5af4225f223838638e2c1595e7 to 4d96d1c560cbc6e3464b9a053e874f4c84fd3500
comment:7 Changed 4 years ago by
- Cc tscrim aapitzsch jdemeyer added
- Status changed from new to needs_review
this seems to be good to go, the latex problem is most probably unrelated
comment:8 Changed 4 years ago by
*ping*
comment:9 Changed 4 years ago by
My opinion is it should return True
for !=
if the classes are distinct, which is a sufficient test for !=
.
comment:10 Changed 4 years ago by
Like I said, I think we should have a general plan for comparing parents. We do have one for elements (the coercion model). As long as we don't have such a plan, I rather wouldn't want to change __cmp__
to __richcmp__
.
comment:11 follow-up: ↓ 12 Changed 4 years ago by
In Parent, we only have _cmp_
which calls __cmp__
and _richcmp
which call _cmp_
I think one should add there a _richcmp_
(possibly replacing the existing _richcmp
) that first tries to call __richcmp__
and then falls back to calling __cmp__
.
Should this be looked at in #22029 or in a new ticket ?
comment:12 in reply to: ↑ 11 Changed 4 years ago by
comment:13 Changed 4 years ago by
- Commit changed from 4d96d1c560cbc6e3464b9a053e874f4c84fd3500 to dbeac6d608d4d26db0404ccb1bd9a6a569c2ab17
comment:14 Changed 4 years ago by
- Reviewers set to Travis Scrimshaw
- Status changed from needs_review to positive_review
I think this will do for now, but the reason we may not see any pickling issues here is because this has a (unsophisticated) way of implementing singleton behavior. So if we change this to an instance of Singleton
, then we don't need the __richcmp__
, but that is for another ticket.
comment:15 Changed 4 years ago by
- Branch changed from u/chapoton/21980 to dbeac6d608d4d26db0404ccb1bd9a6a569c2ab17
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
py3 richcmp in real_double.pyx