Opened 9 years ago

Closed 9 years ago

#13530 closed defect (fixed)

Hide sage/gsl/interpolation/Spline internals

Reported by: jvkersch Owned by: jason, jkantor
Priority: major Milestone: sage-5.5
Component: numerical Keywords: spline, gsl
Cc: kcrisman 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 jvkersch)

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 jvkersch 9 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 9 years ago by jvkersch

  • Description modified (diff)

comment:2 Changed 9 years ago by kcrisman

  • Cc kcrisman added

comment:3 Changed 9 years ago by jvkersch

  • Authors set to Joris Vankerschaver
  • Dependencies set to #13519, #13520
  • Status changed from new to needs_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 9 years ago by tscrim

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 9 years ago by tscrim (previous) (diff)

Changed 9 years ago by jvkersch

comment:5 Changed 9 years ago by jvkersch

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 9 years ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_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 9 years ago by jvkersch

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

comment:8 Changed 9 years ago by jdemeyer

  • Milestone changed from sage-5.4 to sage-5.5

comment:9 Changed 9 years ago by jdemeyer

  • Merged in set to sage-5.5.beta2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.