Opened 5 years ago

Closed 4 years ago

#22649 closed enhancement (fixed)

py3 use richcmp for morphisms of schemes

Reported by: chapoton Owned by:
Priority: major Milestone: sage-8.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:

Status badges

Description

tentative to move towards python3

Change History (18)

comment:1 Changed 5 years ago by chapoton

  • Branch set to u/chapoton/22649
  • Commit set to f301b9e18da69fee529fac9819f1413af4c933f3

New commits:

f301b9etentative use of richcmp for morphisms of schemes

comment:2 Changed 5 years ago by chapoton

almost works..

comment:3 Changed 4 years ago by git

  • Commit changed from f301b9e18da69fee529fac9819f1413af4c933f3 to 04cbb20db3d893ff5611a6a9483f20ab7446f31e

Branch pushed to git repo; I updated commit sha1. New commits:

04cbb20Merge branch 'u/chapoton/22649' in 8.0.b0

comment:4 Changed 4 years ago by git

  • Commit changed from 04cbb20db3d893ff5611a6a9483f20ab7446f31e to 5c5e225979719cccc098fda4e2bd2d5673ca75a4

Branch pushed to git repo; I updated commit sha1. New commits:

5c5e225trac 22649 correct failing doctests

comment:5 Changed 4 years ago by chapoton

  • 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 4 years ago by git

  • Commit changed from 5c5e225979719cccc098fda4e2bd2d5673ca75a4 to 6f72778fb233bb50d42586bce56d8f7d0b1f947b

Branch pushed to git repo; I updated commit sha1. New commits:

6f72778trac 22649 fixing doctests again

comment:7 Changed 4 years ago by chapoton

one doctest failure should be solved by #22743

comment:8 Changed 4 years ago by chapoton

  • 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 4 years ago by chapoton

ok,green bot, please review

comment:10 Changed 4 years ago by tscrim

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:11 Changed 4 years ago by chapoton

  • Cc bhutz cremona added

ok, but who are they ?

comment:12 Changed 4 years ago by chapoton

  • Cc rlmiller added

comment:13 Changed 4 years ago by bhutz

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 follow-up: Changed 4 years ago by chapoton

@cremona, any comments ?

comment:15 in reply to: ↑ 14 Changed 4 years ago by cremona

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 4 years ago by tscrim

  • 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 1-dim 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 4 years ago by tscrim

  • Status changed from needs_review to positive_review

comment:18 Changed 4 years ago by vbraun

  • Branch changed from u/chapoton/22649 to 6f72778fb233bb50d42586bce56d8f7d0b1f947b
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.