#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, GitHub, GitLab) | 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 5 years ago by
- Branch set to u/roed/mark_morphisms_as_coercions
comment:2 Changed 5 years ago by
- Commit set to 6be53f3fbc11bea3ff91918d539085b52e262cca
- Keywords sd86.5 added
- Status changed from new to needs_review
comment:3 Changed 5 years ago by
Just ran tests: they all pass.
comment:4 Changed 5 years ago by
Looks good except for one "Conversion map" that should print as a "Coercion map".
comment:5 Changed 5 years ago by
- Status changed from needs_review to needs_work
comment:6 Changed 5 years ago by
- Branch changed from u/roed/mark_morphisms_as_coercions to u/saraedum/mark_morphisms_as_coercions
comment:7 Changed 5 years ago by
- Commit changed from 6be53f3fbc11bea3ff91918d539085b52e262cca to d8eec2d3e209075c7ab2a96bef095d56f2829aca
- Reviewers set to Julian Rüth
- Status changed from needs_work to needs_review
comment:8 Changed 5 years ago by
- Commit changed from d8eec2d3e209075c7ab2a96bef095d56f2829aca to 2fa81a7203ac37e320acabe5b81461ccb5fab92e
Branch pushed to git repo; I updated commit sha1. New commits:
2fa81a7 | Fix doctests to print exactly as they show up on screen
|
comment:9 Changed 5 years ago by
- Reviewers changed from Julian Rüth to Julian Rüth, David Roe
- Status changed from needs_review to positive_review
comment:11 Changed 5 years ago by
- Commit changed from 2fa81a7203ac37e320acabe5b81461ccb5fab92e to b483c8ef214bbe14d0638406a22b67cb02afa985
Branch pushed to git repo; I updated commit sha1. New commits:
b483c8e | Merge branch 'develop' into t/23211/mark_morphisms_as_coercions
|
comment:12 Changed 5 years ago by
- Status changed from needs_work to needs_review
Fairly trivial merge conflict. Let's wait for the patchbot just to be safe.
New commits:
b483c8e | Merge branch 'develop' into t/23211/mark_morphisms_as_coercions
|
comment:13 Changed 5 years ago by
- Work issues set to waiting for the patchbot → positive review
comment:14 Changed 5 years ago by
- 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 5 years ago by
- 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 5 years ago by
- Commit changed from b483c8ef214bbe14d0638406a22b67cb02afa985 to 78807fa8ab6560e93a85b92d6e28b8943ef2761b
Branch pushed to git repo; I updated commit sha1. New commits:
78807fa | fix doctest errors
|
comment:17 Changed 5 years ago by
- Status changed from needs_work to positive_review
comment:18 Changed 5 years ago by
- 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: ↓ 20 Changed 5 years ago by
- Commit 78807fa8ab6560e93a85b92d6e28b8943ef2761b deleted
you introduced a bad trac role, please review #23526
New commits:
Move _is_coercion from DefaultConvertMap to Map and make it accurate