Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mark morphisms as coercions #23211

Closed
roed314 opened this issue Jun 10, 2017 · 29 comments
Closed

Mark morphisms as coercions #23211

roed314 opened this issue Jun 10, 2017 · 29 comments

Comments

@roed314
Copy link
Contributor

roed314 commented Jun 10, 2017

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.

Component: coercion

Keywords: sd86.5

Author: David Roe, Julian Rüth

Branch: 78807fa

Reviewer: Julian Rüth, David Roe

Issue created by migration from https://trac.sagemath.org/ticket/23211

@roed314 roed314 added this to the sage-8.0 milestone Jun 10, 2017
@roed314
Copy link
Contributor Author

roed314 commented Jun 11, 2017

Branch: u/roed/mark_morphisms_as_coercions

@roed314
Copy link
Contributor Author

roed314 commented Jun 11, 2017

Changed keywords from none to sd86.5

@roed314
Copy link
Contributor Author

roed314 commented Jun 11, 2017

New commits:

6be53f3Move _is_coercion from DefaultConvertMap to Map and make it accurate

@roed314
Copy link
Contributor Author

roed314 commented Jun 11, 2017

Commit: 6be53f3

@roed314
Copy link
Contributor Author

roed314 commented Jun 11, 2017

Author: David Roe

@roed314
Copy link
Contributor Author

roed314 commented Jun 11, 2017

comment:3

Just ran tests: they all pass.

@saraedum
Copy link
Member

comment:4

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

@saraedum
Copy link
Member

@saraedum
Copy link
Member

Changed author from David Roe to David Roe, Julian Rüth

@saraedum
Copy link
Member

Reviewer: Julian Rüth

@saraedum
Copy link
Member

Changed commit from 6be53f3 to d8eec2d

@saraedum
Copy link
Member

New commits:

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 11, 2017

Changed commit from d8eec2d to 2fa81a7

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 11, 2017

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

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

@roed314
Copy link
Contributor Author

roed314 commented Jun 11, 2017

Changed reviewer from Julian Rüth to Julian Rüth, David Roe

@vbraun
Copy link
Member

vbraun commented Jun 15, 2017

comment:10

Merge conflict

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 16, 2017

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

b483c8eMerge branch 'develop' into t/23211/mark_morphisms_as_coercions

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 16, 2017

Changed commit from 2fa81a7 to b483c8e

@saraedum
Copy link
Member

comment:12

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

@saraedum
Copy link
Member

Work Issues: waiting for the patchbot → positive review

@roed314
Copy link
Contributor Author

roed314 commented Jun 16, 2017

comment:14

I just ran tests: they all pass.

@roed314
Copy link
Contributor Author

roed314 commented Jun 16, 2017

Changed work issues from waiting for the patchbot → positive review to none

@vbraun
Copy link
Member

vbraun commented Jun 17, 2017

comment:15
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
**********************************************************************

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 19, 2017

Changed commit from b483c8e to 78807fa

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 19, 2017

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

78807fafix doctest errors

@vbraun
Copy link
Member

vbraun commented Jun 22, 2017

Changed branch from u/saraedum/mark_morphisms_as_coercions to 78807fa

@fchapoton
Copy link
Contributor

comment:19

you introduced a bad trac role, please review #23526

@fchapoton
Copy link
Contributor

Changed commit from 78807fa to none

@roed314
Copy link
Contributor Author

roed314 commented Jul 23, 2017

comment:20

Replying to @fchapoton:

you introduced a bad trac role, please review #23526

Oops! Sorry about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants