Opened 5 years ago
Closed 5 years ago
#22649 closed enhancement (fixed)
py3 use richcmp for morphisms of schemes
Reported by:  chapoton  Owned by:  

Priority:  major  Milestone:  sage8.0 
Component:  python3  Keywords:  
Cc:  tscrim, jdemeyer, bhutz, cremona, rlmiller  Merged in:  
Authors:  Frédéric Chapoton  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  6f72778 (Commits, GitHub, GitLab)  Commit:  6f72778fb233bb50d42586bce56d8f7d0b1f947b 
Dependencies:  Stopgaps: 
Description
tentative to move towards python3
Change History (18)
comment:1 Changed 5 years ago by
 Branch set to u/chapoton/22649
 Commit set to f301b9e18da69fee529fac9819f1413af4c933f3
comment:2 Changed 5 years ago by
almost works..
comment:3 Changed 5 years ago by
 Commit changed from f301b9e18da69fee529fac9819f1413af4c933f3 to 04cbb20db3d893ff5611a6a9483f20ab7446f31e
Branch pushed to git repo; I updated commit sha1. New commits:
04cbb20  Merge branch 'u/chapoton/22649' in 8.0.b0

comment:4 Changed 5 years ago by
 Commit changed from 04cbb20db3d893ff5611a6a9483f20ab7446f31e to 5c5e225979719cccc098fda4e2bd2d5673ca75a4
Branch pushed to git repo; I updated commit sha1. New commits:
5c5e225  trac 22649 correct failing doctests

comment:5 Changed 5 years ago by
 Cc tscrim jdemeyer added
 Status changed from new to needs_info
It seems that the current state of this ticket has the following effect:
Before, comparison of an element of projective space Pn with "other" tried (by itself) to convert "other" to an element of Pn
After, no such conversion is performed. This means that comparison behaves differently, for example P1(0,1) == 0 is not True.
This should rather be fixed by a proper coercion, maybe ?
comment:6 Changed 5 years ago by
 Commit changed from 5c5e225979719cccc098fda4e2bd2d5673ca75a4 to 6f72778fb233bb50d42586bce56d8f7d0b1f947b
Branch pushed to git repo; I updated commit sha1. New commits:
6f72778  trac 22649 fixing doctests again

comment:7 Changed 5 years ago by
one doctest failure should be solved by #22743
comment:8 Changed 5 years ago by
 Status changed from needs_info to needs_review
May be good now. Back to needs review. Waiting for a bot to go through.
comment:9 Changed 5 years ago by
ok,green bot, please review
comment:10 Changed 5 years ago by
I think you might want to cc some of the people who work on schemes to look at this because of the behavior change. I don't know enough about that area of math or how that code is used to say whether or not the change is likely to break stuff.
comment:12 Changed 5 years ago by
 Cc rlmiller added
comment:13 Changed 5 years ago by
I agree that coercion is the right way to go (it's been on the TODO list for awhile now).
As for this change without coercion. I'm not concerned with most of the schemes files as anything being careful about where points live will be fine and those short cuts are much less intuitive when the dimension is >1. I thought the elliptic curve files used the implicit P(0,1)==0 fairly often, but it seems like all tests currently pass. Perhaps John Cremona can comment more on the elliptic curve perspective.
It might be a little more annoying to the user until we get coercion for schemes implemented, but I don't see anything wrong with the new behavior.
comment:14 followup: ↓ 15 Changed 5 years ago by
@cremona, any comments ?
comment:15 in reply to: ↑ 14 Changed 5 years ago by
Replying to chapoton:
@cremona, any comments ?
No. I try to avoid looking at this low level stuff so if tests still pass and it is no slower to test equality of points on elliptic curves then go ahead.
comment:16 Changed 5 years ago by
 Reviewers set to Travis Scrimshaw
Since that is a relatively special case, I don't think it will be used that much. If it is desired, then 1dim projective spaces from their base rings, which would be the simplest way to get the old behavior back. (At least, this is my impression from quickly looking at things.) So I'm setting a positive review and the coercion can be done on a followup ticket.
comment:17 Changed 5 years ago by
 Status changed from needs_review to positive_review
comment:18 Changed 5 years ago by
 Branch changed from u/chapoton/22649 to 6f72778fb233bb50d42586bce56d8f7d0b1f947b
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
tentative use of richcmp for morphisms of schemes