Opened 10 years ago

Closed 10 years ago

#13530 closed defect (fixed)

Hide sage/gsl/interpolation/Spline internals

Reported by: Joris Vankerschaver Owned by: jason, jkantor
Priority: major Milestone: sage-5.5
Component: numerical Keywords: spline, gsl
Cc: Karl-Dieter Crisman Merged in: sage-5.5.beta2
Authors: Joris Vankerschaver Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #13519, #13520 Stopgaps:

Status badges

Description (last modified by Joris Vankerschaver)

The list member function of sage.gsl.interpolation.Spline returns a reference to the spline interpolation points, allowing one to change these points without having the spline be recomputed:

sage: s = spline([(0, 0), (1, 2), (2, 4)]); s
[(0, 0), (1, 2), (2, 4)]
sage: s(1)
2.0
sage: s.list()[1] = (1, 100); s
[(0, 0), (1, 100), (2, 4)]
sage: s(1)
2.0

This issue came up in #12036 and is related to #13520.

Attachments (1)

13530-spline-list-copy.patch (1.2 KB) - added by Joris Vankerschaver 10 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 10 years ago by Joris Vankerschaver

Description: modified (diff)

comment:2 Changed 10 years ago by Karl-Dieter Crisman

Cc: Karl-Dieter Crisman added

comment:3 Changed 10 years ago by Joris Vankerschaver

Authors: Joris Vankerschaver
Dependencies: #13519, #13520
Status: newneeds_review

The patch modifies the list method to return a copy of the interpolation points, as discussed at https://groups.google.com/d/topic/sage-devel/61t8kUyN8L8/discussion

comment:4 Changed 10 years ago by Travis Scrimshaw

Could you change the doctest to show that the list is a copy such as

sage: S = spline([(1,1), (2,3), (4,5)])
sage: L = S.list(); L
[(1, 1), (2, 3), (4, 5)] 
sage: L[2] = (3, 2)
sage: L
[(1, 1), (2, 3), (3, 2)]
sage: S.list()
[(1, 1), (2, 3), (4, 5)]

showing the change and testing it (in a TEST:: block), as well as coping a form of what is in the trac description into the method description?

Edit: Functionally it is good.

Last edited 10 years ago by Travis Scrimshaw (previous) (diff)

Changed 10 years ago by Joris Vankerschaver

comment:5 Changed 10 years ago by Joris Vankerschaver

Thanks for the addendum and sorry for the long wait. I've added the example to the docstring as you requested and moved things around a bit. Is there any difference between starting an example block with TESTS:: versus EXAMPLES::?

comment:6 Changed 10 years ago by Travis Scrimshaw

Reviewers: Travis Scrimshaw
Status: needs_reviewpositive_review

A TESTS:: block is when something is not an example per say, but used in showing some bug has been fixed.

Looks good to me.

comment:7 Changed 10 years ago by Joris Vankerschaver

Ah, yes, that makes sense. Thanks for the clarification and the positive review.

comment:8 Changed 10 years ago by Jeroen Demeyer

Milestone: sage-5.4sage-5.5

comment:9 Changed 10 years ago by Jeroen Demeyer

Merged in: sage-5.5.beta2
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.