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

Upgrade to Pynac-0.6.5 #20475

Closed
rwst opened this issue Apr 20, 2016 · 21 comments
Closed

Upgrade to Pynac-0.6.5 #20475

rwst opened this issue Apr 20, 2016 · 21 comments

Comments

@rwst
Copy link

rwst commented Apr 20, 2016

That Pynac version has:

https://github.com/pynac/pynac/releases/download/pynac-0.6.5/pynac-0.6.5.tar.bz2

Depends on #20312

Component: packages: standard

Author: Ralf Stephan, Benjamin Hackl, Aaditya Thakkar

Branch/Commit: 72d96a5

Reviewer: Eric Gourgoulhon, Volker Braun

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

@rwst rwst added this to the sage-7.2 milestone Apr 20, 2016
@rwst
Copy link
Author

rwst commented Apr 20, 2016

Branch: u/rws/upgrade_to_pynac_0_6_5

@rwst
Copy link
Author

rwst commented Apr 20, 2016

Author: Ralf Stephan, Benjamin Hackl, Aaditya Thakkar

@rwst
Copy link
Author

rwst commented Apr 20, 2016

New commits:

39b85b0new version/chksum
11e2b78doctest changes and a few fixes in support of pynac-0.6.5

@rwst
Copy link
Author

rwst commented Apr 20, 2016

Commit: 11e2b78

@jdemeyer
Copy link

comment:3

I consider this a bug:

         sage: S.<y> = PolynomialRing(RR)
         sage: hermite(3,y)
         8*y^3 - 12*y

@jdemeyer
Copy link

comment:4

Does Sage use Ginac for other polynomial functions?

@rwst
Copy link
Author

rwst commented Apr 21, 2016

Dependencies: #20312

@rwst
Copy link
Author

rwst commented Apr 21, 2016

comment:5

Replying to @jdemeyer:

I consider this a bug:

         sage: S.<y> = PolynomialRing(RR)
         sage: hermite(3,y)
         8*y^3 - 12*y

Indeed, see #20312.

Does Sage use Ginac for other polynomial functions?

No. So?

@jdemeyer
Copy link

comment:6

Replying to @rwst:

No. So?

Because we should look for a solution which would work generally for all polynomial symbolic functions.

For this ticket, I suggest to remove the changes to the hermite function since that's probably not easily fixed.

@rwst
Copy link
Author

rwst commented Apr 21, 2016

comment:7

Replying to @jdemeyer:

Replying to @rwst:

No. So?

Because we should look for a solution which would work generally for all polynomial symbolic functions.

The problem has nothing to do with just polynomial symbolic functions. See the example in #20312 or #20060.

For this ticket, I suggest to remove the changes to the hermite function since that's probably not easily fixed.

I intend to try to fix this (EDIT: I mean #20312), so any hints from you would be welcome.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 23, 2016

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

e06a26cMerge branch 'develop' into t/20475/upgrade_to_pynac_0_6_5
eb5bf9320312: preserving function arg parent, first version
8f91703Merge branch 'u/rws/parent_of_argument_lost_with_functions' of git://trac.sagemath.org/sage into t/20475/upgrade_to_pynac_0_6_5
1986ca5fix to make fixed doctest pass
5c377cc20312: fix for constant results
024924eMerge branch 'u/rws/parent_of_argument_lost_with_functions' of git://trac.sagemath.org/sage into t/20475/upgrade_to_pynac_0_6_5

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 23, 2016

Changed commit from 11e2b78 to 024924e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 6, 2016

Changed commit from 024924e to 72d96a5

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 6, 2016

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

72d96a5Merge branch 'develop' into t/20475/upgrade_to_pynac_0_6_5

@egourgoulhon
Copy link
Member

comment:11

I gave a try and it looks good to me. It would be nice if this is merged in Sage 7.2 before the final release, to correct #20456 (a rather severe bug IMHO). @jdemeyer: do you agree to set the ticket to positive review?

@egourgoulhon
Copy link
Member

Reviewer: Eric Gourgoulhon

@egourgoulhon
Copy link
Member

comment:13

sage -coverage says:

SCORE src/sage/symbolic/pynac.pyx: 97.4% (37 of 38)

Missing doctests:
     * line 2271: def init_function_table()

@rwst
Copy link
Author

rwst commented May 6, 2016

comment:14

Come on, we didn't have a doctest for this since it was added 6 years ago and I don't think we need one. Most of these functions aren't used anyway because the Function code already bypasses Pynac with FP evaluation, and Pynac itself does not need it.

@egourgoulhon
Copy link
Member

comment:15

Replying to @rwst:

Come on, we didn't have a doctest for this since it was added 6 years ago and I don't think we need one. Most of these functions aren't used anyway because the Function code already bypasses Pynac with FP evaluation, and Pynac itself does not need it.

OK, I see.

@vbraun
Copy link
Member

vbraun commented May 6, 2016

Changed reviewer from Eric Gourgoulhon to Eric Gourgoulhon, Volker Braun

@vbraun
Copy link
Member

vbraun commented May 6, 2016

Changed branch from u/rws/upgrade_to_pynac_0_6_5 to 72d96a5

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

5 participants