Opened 8 months ago

Closed 6 months ago

Last modified 5 months ago

#31716 closed enhancement (fixed)

more scalar conversions

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-9.4
Component: basic arithmetic Keywords:
Cc: Merged in:
Authors: Vincent Delecroix Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: e40555a (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by vdelecroix)

Following #10147, we extend conversion to scalars for polynomials, multi-polynomials and fraction fields.

The attached branch allows the following construction

sage: x = polygen(QQ)
sage: A.<u> = NumberField(x^3 - 2)
sage: A((x + 3) / (2*x - 1))
14/15*u^2 + 7/15*u + 11/15

See the original report from asksage.

Change History (14)

comment:1 Changed 8 months ago by vdelecroix

  • Authors set to Vincent Delecroix
  • Branch set to u/vdelecroix/31716
  • Commit set to 76fdce3796a52c5129d75376278f7a64e68807e0
  • Status changed from new to needs_review

New commits:

76fdce331716: more scalar conversions

comment:2 Changed 8 months ago by vdelecroix

  • Description modified (diff)
  • Status changed from needs_review to needs_work

comment:3 Changed 8 months ago by git

  • Commit changed from 76fdce3796a52c5129d75376278f7a64e68807e0 to 9dbc024025d5447748abe30371827204739da62b

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

e2a22cb31716: more scalar conversions
9dbc02431716: fix doctests

comment:4 Changed 8 months ago by vdelecroix

  • Status changed from needs_work to needs_review

comment:5 Changed 8 months ago by git

  • Commit changed from 9dbc024025d5447748abe30371827204739da62b to 4e245da21ba5a8557995a47bde15a0dac1f386b9

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

4e245da31716: more doctests

comment:6 Changed 8 months ago by vdelecroix

  • Description modified (diff)

comment:7 Changed 8 months ago by tscrim

  • Reviewers set to Travis Scrimshaw

LGTM except you lost the doctest for MPolynomial._integer_. Once you add one, you can set a positive review.

comment:8 Changed 8 months ago by git

  • Commit changed from 4e245da21ba5a8557995a47bde15a0dac1f386b9 to 779b1ad6cc8df67c463865c8cad1c4305e8dace9

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

779b1admore tests

comment:9 Changed 8 months ago by git

  • Commit changed from 779b1ad6cc8df67c463865c8cad1c4305e8dace9 to e40555aff8c29f8dfb771273416cf2c24fb84aa3

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

e40555a31716: remove MPolynomial._integer_

comment:10 Changed 8 months ago by vdelecroix

Thanks for spotting that. My goal was actually to *remove* the _integer_ method (done in e40555a). I copied over the tests in _scalar_conversion (779b1ad).

comment:11 Changed 8 months ago by tscrim

Once the patchbot comes back green, positive review.

comment:12 Changed 7 months ago by vdelecroix

  • Status changed from needs_review to positive_review

green. Thanks.

comment:13 Changed 6 months ago by vbraun

  • Branch changed from u/vdelecroix/31716 to e40555aff8c29f8dfb771273416cf2c24fb84aa3
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:14 Changed 5 months ago by mmezzarobba

  • Commit e40555aff8c29f8dfb771273416cf2c24fb84aa3 deleted

I suspect (but have not checked) that #32078 was caused by the changes here.

Note: See TracTickets for help on using tickets.