Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#23211 closed enhancement (fixed)

Mark morphisms as coercions

Reported by: roed Owned by:
Priority: minor Milestone: sage-8.0
Component: coercion Keywords: sd86.5
Cc: Merged in:
Authors: David Roe, Julian Rüth Reviewers: Julian Rüth, David Roe
Report Upstream: N/A Work issues:
Branch: 78807fa (Commits) Commit:
Dependencies: Stopgaps:

Description

Currently, DefaultCovertMaps have an attribute _is_coercion that is unused. I propose moving it up to Map and setting it when coercion maps are created.

Change History (20)

comment:1 Changed 2 years ago by roed

  • Branch set to u/roed/mark_morphisms_as_coercions

comment:2 Changed 2 years ago by roed

  • Authors set to David Roe
  • Commit set to 6be53f3fbc11bea3ff91918d539085b52e262cca
  • Keywords sd86.5 added
  • Status changed from new to needs_review

New commits:

6be53f3Move _is_coercion from DefaultConvertMap to Map and make it accurate

comment:3 Changed 2 years ago by roed

Just ran tests: they all pass.

comment:4 Changed 2 years ago by saraedum

Looks good except for one "Conversion map" that should print as a "Coercion map".

comment:5 Changed 2 years ago by saraedum

  • Status changed from needs_review to needs_work

comment:6 Changed 2 years ago by saraedum

  • Branch changed from u/roed/mark_morphisms_as_coercions to u/saraedum/mark_morphisms_as_coercions

comment:7 Changed 2 years ago by saraedum

  • Authors changed from David Roe to David Roe, Julian Rüth
  • Commit changed from 6be53f3fbc11bea3ff91918d539085b52e262cca to d8eec2d3e209075c7ab2a96bef095d56f2829aca
  • Reviewers set to Julian Rüth
  • Status changed from needs_work to needs_review

New commits:

7dac7e2Fix doctest
9a9e437Print default "coerce" maps as "Coercion"
d8eec2dFixed doctests

comment:8 Changed 2 years ago by git

  • Commit changed from d8eec2d3e209075c7ab2a96bef095d56f2829aca to 2fa81a7203ac37e320acabe5b81461ccb5fab92e

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

2fa81a7Fix doctests to print exactly as they show up on screen

comment:9 Changed 2 years ago by roed

  • Reviewers changed from Julian Rüth to Julian Rüth, David Roe
  • Status changed from needs_review to positive_review

comment:10 Changed 2 years ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:11 Changed 2 years ago by git

  • Commit changed from 2fa81a7203ac37e320acabe5b81461ccb5fab92e to b483c8ef214bbe14d0638406a22b67cb02afa985

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

b483c8eMerge branch 'develop' into t/23211/mark_morphisms_as_coercions

comment:12 Changed 2 years ago by saraedum

  • Status changed from needs_work to needs_review

Fairly trivial merge conflict. Let's wait for the patchbot just to be safe.


New commits:

b483c8eMerge branch 'develop' into t/23211/mark_morphisms_as_coercions

comment:13 Changed 2 years ago by saraedum

  • Work issues set to waiting for the patchbot → positive review

comment:14 Changed 2 years ago by roed

  • Status changed from needs_review to positive_review
  • Work issues waiting for the patchbot → positive review deleted

I just ran tests: they all pass.

comment:15 Changed 2 years ago by vbraun

  • Status changed from positive_review to needs_work
sage -t --long --warn-long 73.0 src/doc/en/thematic_tutorials/coercion_and_categories.rst
**********************************************************************
File "src/doc/en/thematic_tutorials/coercion_and_categories.rst", line 826, in doc.en.thematic_tutorials.coercion_and_categories
Failed example:
    P1.coerce_map_from(P2)
Expected:
    Conversion map:
      From: Multivariate Polynomial Ring in w, v over Integer Ring
      To:   Multivariate Polynomial Ring in v, w over Rational Field
Got:
    Coercion map:
      From: Multivariate Polynomial Ring in w, v over Integer Ring
      To:   Multivariate Polynomial Ring in v, w over Rational Field
**********************************************************************
1 item had failures:
   1 of 192 in doc.en.thematic_tutorials.coercion_and_categories
    [191 tests, 1 failure, 0.72 s]
----------------------------------------------------------------------
sage -t --long --warn-long 73.0 src/doc/en/thematic_tutorials/coercion_and_categories.rst  # 1 doctest failed
----------------------------------------------------------------------
Total time for all tests: 2.1 seconds
    cpu time: 0.7 seconds
    cumulative wall time: 0.7 seconds

and

sage -t --long --warn-long 73.0 src/doc/en/reference/coercion/index.rst  # 1 doctest failed
Running doctests with ID 2017-06-17-14-11-34-0b4119bb.
Git branch: develop
Using --optional=mpir,python2,sage
Doctesting 1 file.
sage -t --long --warn-long 73.0 src/doc/en/reference/coercion/index.rst
**********************************************************************
File "src/doc/en/reference/coercion/index.rst", line 223, in doc.en.reference.coercion.index
Failed example:
    cm.explain(ZZ['x','y'], QQ['x'])
Expected:
    Coercion on left operand via
       Conversion map:
         From: Multivariate Polynomial Ring in x, y over Integer Ring
         To:   Multivariate Polynomial Ring in x, y over Rational Field
    Coercion on right operand via
       Conversion map:
         From: Univariate Polynomial Ring in x over Rational Field
         To:   Multivariate Polynomial Ring in x, y over Rational Field
    Arithmetic performed after coercions.
    Result lives in Multivariate Polynomial Ring in x, y over Rational Field
    Multivariate Polynomial Ring in x, y over Rational Field
Got:
    Coercion on left operand via
        Coercion map:
          From: Multivariate Polynomial Ring in x, y over Integer Ring
          To:   Multivariate Polynomial Ring in x, y over Rational Field
    Coercion on right operand via
        Coercion map:
          From: Univariate Polynomial Ring in x over Rational Field
          To:   Multivariate Polynomial Ring in x, y over Rational Field
    Arithmetic performed after coercions.
    Result lives in Multivariate Polynomial Ring in x, y over Rational Field
    Multivariate Polynomial Ring in x, y over Rational Field
**********************************************************************

comment:16 Changed 2 years ago by git

  • Commit changed from b483c8ef214bbe14d0638406a22b67cb02afa985 to 78807fa8ab6560e93a85b92d6e28b8943ef2761b

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

78807fafix doctest errors

comment:17 Changed 2 years ago by saraedum

  • Status changed from needs_work to positive_review

comment:18 Changed 2 years ago by vbraun

  • Branch changed from u/saraedum/mark_morphisms_as_coercions to 78807fa8ab6560e93a85b92d6e28b8943ef2761b
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:19 follow-up: Changed 2 years ago by chapoton

  • Commit 78807fa8ab6560e93a85b92d6e28b8943ef2761b deleted

you introduced a bad trac role, please review #23526

comment:20 in reply to: ↑ 19 Changed 2 years ago by roed

Replying to chapoton:

you introduced a bad trac role, please review #23526

Oops! Sorry about that.

Note: See TracTickets for help on using tickets.