Opened 14 years ago

Closed 14 years ago

#5413 closed defect (fixed)

[with patch, positive review] deprecate substitution via __call__ w/ unnamed arguments

Reported by: Carl Witty Owned by: Burcin Erocal
Priority: major Milestone: sage-3.4.1
Component: calculus Keywords:
Cc: Jason Grout Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

As discussed on sage-devel here: http://groups.google.com/group/sage-devel/browse_thread/thread/b1a03f8fc8ae8fcd/553773d7ba600ae7#553773d7ba600ae7

I added deprecation warnings to the four affected call functions (two for symbolic values, one for equations, one for matrices), and fixed almost all the warnings in all the doctests other than the doctests specifically for those call methods.

There's one set of warnings that I didn't figure out how to fix (in piecewise.py), so I just added the warning to the expected output for now.

Attachments (3)

deprecate-symbolic-unnamed-call.patch (26.2 KB) - added by Carl Witty 14 years ago.
trac5413-deprecate-symbolic-unnamed-call.patch (49.3 KB) - added by Carl Witty 14 years ago.
trac_5413-reviewer.patch (820 bytes) - added by Michael Abshoff 14 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 14 years ago by Michael Abshoff

Summary: [with patch, needs review] deprecate substitution via __call__ w/ unnamed arguments[with patch, needs review, needs doctest fix] deprecate substitution via __call__ w/ unnamed arguments

Two trivial doctest failures:

sage -t -long devel/sage/doc/en/constructions/calculus.rst # 1 doctests failed
sage -t -long devel/sage/doc/fr/tutorial/tour_algebra.rst # 1 doctests failed

Cheers,

Michael

comment:2 Changed 14 years ago by Jason Grout

Cc: Jason Grout added

Changed 14 years ago by Carl Witty

comment:3 Changed 14 years ago by Carl Witty

Summary: [with patch, needs review, needs doctest fix] deprecate substitution via __call__ w/ unnamed arguments[with patch, needs review] deprecate substitution via __call__ w/ unnamed arguments

Oops, forgot to doctest the documentation. I've replaced my original patch with a new one that also fixes these two doctest failures.

comment:4 Changed 14 years ago by Jason Grout

This is an (email) reminder to myself to review this to get it in.

comment:5 Changed 14 years ago by Jason Grout

This ticket probably also affects #5093.

comment:6 Changed 14 years ago by Burcin Erocal

Thanks for working on this Carl.

I only read the patch, didn't test it yet. Explicitly having to use .function() in two places bothers me:

  • for each component of Piecewise
  • in the call to generate_plot_points

Perhaps we should modify these to handle non function arguments more gracefully after this change.

I understand that this might require a new syntax to construct Piecewise objects. One option is to add a new parameter, which includes an ordered list of variables. E.g.,

    sage: Piecewise([[(0,1),x^2],[(1,2),5-x^2]] ,[x])

or

    sage: f = Piecewise([[(-1,1),1/2+x-x^3 + y]],[x,y])

Comments?

comment:7 Changed 14 years ago by Jason Grout

The above comment for Piecewise would probably also apply to symbolic matrices and vectors, right? (well, at least, as soon as symbolic vectors are callable, anyway). I think that Burcin's proposal is okay, but doesn't extend very well to matrices and vectors, since I'd hate to mess up the standard matrix/vector constructors. I'm don't know a better way to extend this to matrices and vectors, though. Maybe we ought to just deprecate calling a symbolic matrix without keyword arguments, and just implement the callable symbolic vector with the same restrictions.

comment:8 Changed 14 years ago by Carl Witty

The patch already does deprecate calling a symbolic matrix without keyword arguments.

comment:9 Changed 14 years ago by Jason Grout

Yes. I thought Burcin's comment was about a syntax to avoid the .function() call. That's what I was commenting on.

comment:10 Changed 14 years ago by Carl Witty

Summary: [with patch, needs review] deprecate substitution via __call__ w/ unnamed arguments[with patch, needs work] deprecate substitution via __call__ w/ unnamed arguments

After discussions on IRC and sage-devel (http://groups.google.com/group/sage-devel/browse_thread/thread/97cdf80edb089481/73e8e6659c09e1d9#73e8e6659c09e1d9) we've decided on a much more extensive deprecation scheme. So it's not worth reviewing the current patch. (I should have a new patch within a few days.)

Changed 14 years ago by Carl Witty

comment:11 Changed 14 years ago by Carl Witty

Summary: [with patch, needs work] deprecate substitution via __call__ w/ unnamed arguments[with patch, needs review] deprecate substitution via __call__ w/ unnamed arguments

I've posted a patch that implements the more extensive deprecation described here: http://groups.google.com/group/sage-devel/browse_thread/thread/97cdf80edb089481#

The patch now depends on #5093.

Apply only the second patch.

comment:12 Changed 14 years ago by Jason Grout

This is really odd:

[19:53] <jason> sage: g(x)=sin
[19:53] <jason> sage: g(3)
[19:53] <jason> sin(3)
[19:53] <jason> sage: g(x)=sin+x
[19:53] <jason> sage: g(3)
[19:53] <jason> sin + 3
[19:54] <jason> but
[19:54] <jason> sage: g(x)=sin+cos; g(3)
[19:54] <jason> sin + cos

comment:13 Changed 14 years ago by Burcin Erocal

Jason, I get the same behavior without Carl's patch on Sage-3.4. I suggest moving that to a separate ticket. We could also argue if that is valid syntax there.

I give a positive review to this. It would be good to get this in an alpha and let more people play with it. Thanks again Carl.

Cheers, Burcin

comment:14 Changed 14 years ago by Burcin Erocal

Summary: [with patch, needs review] deprecate substitution via __call__ w/ unnamed arguments[with patch, positive review] deprecate substitution via __call__ w/ unnamed arguments

comment:12 is now #5607.

comment:15 Changed 14 years ago by Jason Grout

I agree that we should get this into the alpha so it gets wider exposure. (providing doctests pass) positive review from me too.

comment:16 Changed 14 years ago by Michael Abshoff

There is one tiny doctest issue left - at least in my merge tree :)

sage -t -long "devel/sage/doc/en/constructions/calculus.rst"
**********************************************************************
File "/scratch/mabshoff/sage-3.4.1.alpha0/devel/sage/doc/en/constructions/calculus.rst", line 222:
    sage: f.integral()
Expected:
    Piecewise defined function with 2 parts, [[(0, 1), x^3/3], [(1, 2), (15*x - x^3)/3 - 13/3]]
Got:
    Piecewise defined function with 2 parts, [[(0, 1), x |--> x^3/3], [(1, 2), x |--> (15*x - x^3)/3 - 13/3]]
**********************************************************************
1 items had failures:
   1 of   9 in __main__.example_8
***Test Failed*** 1 failures.
For whitespace errors, see the file /scratch/mabshoff/sage-3.4.1.alpha0/tmp/.doctest_calculus.py
	 [4.3 s]
exit code: 1024

I am fixing this with a reviewer patch.

Cheers,

Michael

Changed 14 years ago by Michael Abshoff

Attachment: trac_5413-reviewer.patch added

comment:17 Changed 14 years ago by Michael Abshoff

Resolution: fixed
Status: newclosed

Merged trac5413-deprecate-symbolic-unnamed-call.patch and trac_5413-reviewer.patch in Sage 3.4.1.alpha0.

Cheers,

Michael

Note: See TracTickets for help on using tickets.