Opened 3 years ago

Closed 3 years ago

#27088 closed enhancement (fixed)

better conversion of univariate polynomials from Fricas

Reported by: chapoton Owned by:
Priority: major Milestone: sage-8.7
Component: interfaces: optional Keywords: fricas
Cc: mantepse Merged in:
Authors: Martin Rubey, Frédéric Chapoton Reviewers: Martin Rubey, Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: e66a5e3 (Commits, GitHub, GitLab) Commit: e66a5e399e63e20d1d1f2bb1973116dbb39a2d0d
Dependencies: Stopgaps:

Status badges

Description


Change History (19)

comment:1 Changed 3 years ago by chapoton

  • Branch set to u/chapoton/27088
  • Commit set to 445f659cc93290542176709c9774ef7bdf3ba2cc

New commits:

445f659better conversions of univariate polynomials from Fricas

comment:2 Changed 3 years ago by git

  • Commit changed from 445f659cc93290542176709c9774ef7bdf3ba2cc to 90225c3e607961e98116340bcb1e8548c131001c

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

90225c3missing tags for new doctests

comment:3 Changed 3 years ago by chapoton

  • Status changed from new to needs_review

comment:4 Changed 3 years ago by mantepse

Thanks for adding this. It is unfortunate that the FriCAS domain SingletonAsOrderedSet does not have an InputForm, which implies that UnivariatePolynomial and SparseUnivariatePolynomial also don't. Maybe this will change at some point.

Anyway, currently you are not testing this:

sage: x = polygen(QQ, 'x')
sage: fricas(x+3)
x + 3
sage: fricas(x+3).domainOf()
Polynomial(Integer())

I think you want

sage: f = fricas("(y^2+3)::UP(y, INT)").sage(); f
y^2 + 3
sage: f.parent()
Univariate Polynomial Ring in y over Integer Ring

comment:5 Changed 3 years ago by git

  • Commit changed from 90225c3e607961e98116340bcb1e8548c131001c to a28f07c02043950b7daf17257a301a82365e90fd

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

a28f07ctrac 27088 more doctests

comment:6 Changed 3 years ago by chapoton

I have added some doctests..

comment:7 Changed 3 years ago by mantepse

It doesn't quite work yet, but that may actually belong to a different ticket:

sage: fricas("(y^2+3*x)::UP(y, UP(x, INT))").sage()
...
NotImplementedError: The translation of FriCAS type (UnivariatePolynomial x (Integer)) to sage is not yet implemented.

sage: fricas("(y^2+sqrt 3)::UP(y, AN)").sage()
...
TypeError: Illegal initializer for algebraic number

sage: fricas("(y^2+sqrt 3)::POLY(AN)").sage()
  File "<string>", line unknown
SyntaxError: Malformed expression

That's a bit strange, because I thought I have tested such constructions!

comment:8 Changed 3 years ago by git

  • Commit changed from a28f07c02043950b7daf17257a301a82365e90fd to 39f440dc918c6032e8f0338add9936d08b5cfaff

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

39f440dbetter still conversion of polynomials from Fricas to Sage

comment:9 Changed 3 years ago by git

  • Commit changed from 39f440dc918c6032e8f0338add9936d08b5cfaff to cc5486e30392efd561b07a0ab68bcdf004ab75a9

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

cc5486etrac 27088 forgotten tag added

comment:10 Changed 3 years ago by chapoton

Martin, is this good enough ?

comment:11 Changed 3 years ago by mantepse

I'm on it, please excuse the delay!

comment:12 Changed 3 years ago by mantepse

  • Branch changed from u/chapoton/27088 to public/27088

comment:13 Changed 3 years ago by mantepse

  • Commit changed from cc5486e30392efd561b07a0ab68bcdf004ab75a9 to 59ffe09eddadb7f56cf327e3be286b4635b5b798

Excellent! I tested this a fair bit, modified one test to check nested types and I made some very cosmetic changes.

If all this is OK for you, please set it to positive review on my behalf! And thanks again!

I'll deal with the Polynomial bug in a separate ticket, but probably I'll wait for fricas 1.3.5 to come out first, to avoid more merge conflicts with #25899


New commits:

59ffe09modify a test

comment:14 Changed 3 years ago by mantepse

  • Reviewers set to Martin Rubey

comment:15 Changed 3 years ago by chapoton

missing one optional fricas here: sage: fricas(0)._get_sage_type(m)

comment:16 Changed 3 years ago by git

  • Commit changed from 59ffe09eddadb7f56cf327e3be286b4635b5b798 to e66a5e399e63e20d1d1f2bb1973116dbb39a2d0d

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

e66a5e3forgot tag

comment:17 Changed 3 years ago by mantepse

Oops, merci beaucoup !

comment:18 Changed 3 years ago by chapoton

  • Authors changed from Frédéric Chapoton to Martin Rubey, Frédéric Chapoton
  • Reviewers changed from Martin Rubey to Martin Rubey, Frédéric Chapoton
  • Status changed from needs_review to positive_review

ok, let it be. Danke schön !

comment:19 Changed 3 years ago by vbraun

  • Branch changed from public/27088 to e66a5e399e63e20d1d1f2bb1973116dbb39a2d0d
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.