Opened 7 years ago
Closed 6 years ago
#17447 closed enhancement (fixed)
Clarify and complete documentation of function()
Reported by:  schymans  Owned by:  

Priority:  major  Milestone:  sage6.10 
Component:  documentation  Keywords:  
Cc:  nbruin, kcrisman, tmonteil, zimmerma  Merged in:  
Authors:  Nils Bruin, Ralf Stephan  Reviewers:  Ralf Stephan, Nils Bruin 
Report Upstream:  N/A  Work issues:  
Branch:  7c029f3 (Commits, GitHub, GitLab)  Commit:  7c029f3c98032a985c2ca0329dfe940271e9b86b 
Dependencies:  Stopgaps: 
Description (last modified by )
The documentation of function() is incomplete and confusing. For example, none of the methods described in http://www.sagemath.org/doc/reference/calculus/sage/symbolic/function_factory.html show up when I type:
function?
The distinction between
f = function('f')
and
f = function('f', x)
is also not documented. See also #17445 for sources of confusion. Need to explain what happens in the second case above (f = function('f',x)
). A symbolic function f
is first created and then overwritten by the expression f
?
See the following example, where f is first an expression, then becomes redefined to a function in the background, but does not contain any information about its variables.
sage: f = function('f', x); print type(f) <type 'sage.symbolic.expression.Expression'> sage: fx = function('f',x); print type(f) <class 'sage.symbolic.function_factory.NewSymbolicFunction'> sage: f.variables() () sage: fx.variables() (x,)
See http://ask.sagemath.org/question/9932/howtosubstituteafunctionwithinderivatives/?answer=14752#postid14752 for a possible start at how to explain this, at least for those writing this.
Change History (33)
comment:1 Changed 7 years ago by
 Description modified (diff)
comment:2 Changed 7 years ago by
 Description modified (diff)
comment:3 followup: ↓ 26 Changed 7 years ago by
comment:4 Changed 7 years ago by
 Component changed from symbolics to documentation
comment:5 Changed 7 years ago by
Awesome, thank you, Nils. These things always confused me, so I'm relieved to hear that they should be avoided! Even the documentation of var is full of such examples:
sage: x = var('x', domain=RR); x; x.conjugate() x x sage: y = var('y', domain='real'); y.conjugate() y sage: y = var('y', domain='positive'); y.abs() y Custom latex expression can be assigned to variable: sage: x = var('sui', latex_name="s_{u,i}"); x._latex_() 's_{u,i}'
Should we change these examples, too?
comment:6 Changed 7 years ago by
 Branch set to u/nbruin/clarify_and_complete_documentation_of_function__
Don't base any other work on this branch just yet, since it's likely to be rebased. I have deprecated the use of function('f',x)
in favour of using function('f')(x)
. It's hardly longer to type and much less ambiguous. Otherwise, it seems that the documentation here is actually pretty accurate, apart from being terse. As soon as the objects returned by function('f')
are consistent, I think users will be forced to learn the (now clear) semantics pretty quickly.
The object returned by function(f)
doesn't allow symbolic operations on it, so people will quickly find they need to evaluate it in order to get a symbolic expression.
I haven't changed all doctests to take into account the deprecation, but I've done some files to show the impact of the change (seems relatively minor).
comment:7 followup: ↓ 8 Changed 7 years ago by
 Commit set to fa3ebd6645eb2cae45c2218628eaa1d984f16506
 Status changed from new to needs_review
Not really ready for *review* but is ready for input. And is mainly ready for someone else to rewrite documentation properly. People CC'd on this ticket look like good candidates to do so.
New commits:
b41248b  trac 17447: Deprecation of function('f',x) in favour of function('f')(x)

fa3ebd6  trac 17447: Doctest changes to reflect deprecation of function('f',x)

note that the doctest that apparently got removed in b41248b in reality gets moved to var.pyx because it tests the toplevel function
, not the library function
comment:8 in reply to: ↑ 7 ; followup: ↓ 9 Changed 7 years ago by
Replying to nbruin:
Not really ready for *review* but is ready for input. And is mainly ready for someone else to rewrite documentation properly. People CC'd on this ticket look like good candidates to do so.
New commits:
b41248b trac 17447: Deprecation of function('f',x) in favour of function('f')(x)
fa3ebd6 trac 17447: Doctest changes to reflect deprecation of function('f',x)
note that the doctest that apparently got removed in b41248b in reality gets moved to var.pyx because it tests the toplevel
function
, not the libraryfunction
This makes it a lot clearer to me now. Just a minor thing: In one of the examples, I would replace
sage: cr = function('cr') sage: f = cr(a)
by
sage: function('cr') sage: f = cr(a)
which in my understanding does the same but is shorter and makes clear that function('cr')
does already create a symbolic function called cr
. Otherwise, one may be surprised to create two functions by typing e.g. cr1 = function('cr')
.
comment:9 in reply to: ↑ 8 Changed 7 years ago by
Replying to schymans:
This makes it a lot clearer to me now. Just a minor thing: In one of the examples, I would replace
sage: cr = function('cr') sage: f = cr(a)
At least one occurrence of that is in the doctest of sage.symbolic.function_factory.function
, which (now) goes out of its way to import that function, which does not have the sideeffect of mutating the global scope, as documented. So the assignment is actually necessary.
Another independent point: the toplevel function
does refer to sage.symbolic.function_factory.function
, but perhaps should do so more explicitly, if possible with a hyperlink. The examples on the latter are a little more elaborate, so someone who wants to read up on function
(and would probably find the toplevel documentation first via sage: function?
). I don't know how to make hyperlinks to other documentation in sage's docstrings.
comment:10 followup: ↓ 11 Changed 7 years ago by
 Status changed from needs_review to needs_work
There are broken doctests in the 'doc' folder
sage t long prep/Quickstarts/DifferentialEquations.rst # 1 doctest failed sage t long tutorial/tour_algebra.rst # 1 doctest failed sage t long constructions/calculus.rst # 1 doctest failed
Nathann
comment:11 in reply to: ↑ 10 ; followup: ↓ 12 Changed 7 years ago by
Replying to ncohen:
There are broken doctests in the 'doc' folder
Yes, and that is not the only place. First we need to see if there is any good reason why function('f',x)
is preferable over function('f')(x)
, of if we can get away with deprecating this confusing construction. And then someone should take a look at the doc to see if it needs further improvement. You're on a doc revamping binge, so this might just be the task for you!
comment:12 in reply to: ↑ 11 Changed 7 years ago by
You're on a doc revamping binge, so this might just be the task for you!
I am afraid that you will have to find somebody else. I never used Sage's symbolics.
Nathann
comment:13 Changed 7 years ago by
 Cc tmonteil added
comment:14 Changed 7 years ago by
 Branch changed from u/nbruin/clarify_and_complete_documentation_of_function__ to public/17447
comment:15 Changed 7 years ago by
 Commit changed from fa3ebd6645eb2cae45c2218628eaa1d984f16506 to e6a35343e5f712b88a5a9a80abd4e9302879c187
comment:16 Changed 7 years ago by
 Commit changed from e6a35343e5f712b88a5a9a80abd4e9302879c187 to 4585e74063e81a8c332a500d016e837069edf8eb
Branch pushed to git repo; I updated commit sha1. New commits:
4585e74  17447: fix rest of doctests

comment:17 Changed 7 years ago by
 Reviewers set to Ralf Stephan
These are the remaining doctest fixes coming from make ptestlong
. I have reviewed and found fine the commits up to mine, so following reviewers can skip that.
And then someone should take a look at the doc to see if it needs further improvement.
comment:18 followups: ↓ 19 ↓ 21 Changed 7 years ago by
 Cc zimmerma added
I suspect that Zimmerman's calculus book uses this stuff quite extensively, so deprecating/changing the current behaviour will likely lead to pushback from him, and probably rightly so.
Cc:ing him for that reason. Though we have already had a few other discussions on Trac about the need to update our tests while not maintaining exact compatibility. Since they don't (yet) raise errors, but apparently only the deprecation warning, should we maybe update the tests (in that part only) to have the deprecation warning returned? Note that I don't believe the deprecation warning is even doctested, which is a nono :)
Also, does the current branch actually "clarify and complete documentation of function"? It looks like it mostly fixes doctests.
Another change not doctested is
if is_SymbolicVariable(dvar):  raise ValueError("You have to declare dependent variable as a function, eg. y=function('y',x)") + raise ValueError("You have to declare dependent variable as a function evaluated at the independent variable, eg. y=function('y')(x)")
which I think happens twice.
I'm still not sure I even understand some of the subtle differences. Are there occasions where the old behavior was "right" in the sense that one wanted that returned, and have we shown how to get that object? What about the ask.sagemath question?
All that to say that nonetheless it will be great to have a unified interface on this, if that is really the right thing to do, which from the comments it apparently is.
comment:19 in reply to: ↑ 18 Changed 7 years ago by
Replying to kcrisman:
Also, does the current branch actually "clarify and complete documentation of function"? It looks like it mostly fixes doctests.
I think there are two things that are/were confusing:
function('f',x)
returns something that is not, according to our definition, a function (e.g., it cannot be evaluated ("called") in a nonambiguous way and hence triggers a deprecation warning)). The toplevel
function
makes a binding in the global namespace and returns a value. The two were different.
With the proposed patch, function('f',x)
is deprecated, so number 1 gets eliminated and number 2 is less confusing because, while it still injects something in the namespace, the value it returns is always the same as what it injects.
When I read the documentation of sage.symbolic.function_factory.function
, I actually thought it described what happens fairly correctly, and it has good examples. Hence I did not feel the need to add to it. The toplevel function
documentation is rather sparse, so that could use expansion. However, we do not want to repeat ourselves, so perhaps it should just contain a clear pointer (hyperlink?) to the documentation of sage.symbolic.function_factory.function
. I don't know how to do that, so please go ahead and improve that bit. You are excellently qualified to produce extensive and understandable documentation for the target audience of these functions.
Another change not doctested is
if is_SymbolicVariable(dvar):  raise ValueError("You have to declare dependent variable as a function, eg. y=function('y',x)") + raise ValueError("You have to declare dependent variable as a function evaluated at the independent variable, eg. y=function('y')(x)")which I think happens twice.
These messages weren't doctested before either. If you feel the projects benefits from such administrations, go ahead.
I'm still not sure I even understand some of the subtle differences. Are there occasions where the old behavior was "right" in the sense that one wanted that returned, and have we shown how to get that object?
In my opinion: no. In all cases, function('f')(x)
is much clearer and not much longer than function('f',x)
. There are *many* tickets and questions about people getting tripped up by the fact that a function called function doesn't return a function.
What about the ask.sagemath question?
In my opinion, that's another thing to document. However, the confusion about function not returning a function definitely contributed to the confusion about how to use substitute_function.
comment:20 Changed 7 years ago by
The toplevel function documentation is rather sparse, so that could use expansion.
Yes, this is what I meant. Hyperlink is tricky  not per se, but in command line and even sagenb it's not as useful. So I figure at least a little more in the toplevel and then the hyperlink (assuming that file is in fact already in the documentation!).
You are excellently qualified to produce extensive and understandable documentation for the target audience of these functions.
That is very kind, I will see; this one is lower on my priority list but definitely important.
If you feel the projects benefits from such administrations, go ahead.
Well, I've found it does. I guess I will ask you to review it if I do that :)
In my opinion, that's another thing to document. However, the confusion about function not returning a function definitely contributed to the confusion about how to use substitute_function.
Yes.
comment:21 in reply to: ↑ 18 Changed 7 years ago by
Replying to kcrisman:
I suspect that Zimmerman's calculus book uses this stuff quite extensively, so deprecating/changing the current behaviour will likely lead to pushback from him, and probably rightly so.
Cc:ing him for that reason.
thanks, however I gave up trying to maintain compatibility with the book, since there are already many examples from the book that we put as doctests in Sage, and which were changed afterwards...
comment:22 followup: ↓ 23 Changed 7 years ago by
Here is another example of something confusing.
http://ask.sagemath.org/question/26114/whyisbasicarithmeticdisallowedonsymbolicfunctions/
comment:23 in reply to: ↑ 22 ; followups: ↓ 24 ↓ 25 Changed 7 years ago by
Replying to kcrisman:
Here is another example of something confusing.
http://ask.sagemath.org/question/26114/whyisbasicarithmeticdisallowedonsymbolicfunctions/
I think that's another issue. The problem there is that global namespace var
and function
return a value as well as insert something into the namespace. That's counter to python custom (routines that mutate state normally return None
. Compare L.sort()
and sorted(L)
. There are exceptions: L.pop()
).
Initially it seems pedantic to enforce such rules in a computer algebra system as well, but the many problems it causes suggests it's not. Can we have declare_var('x,y')
and declare_function('f')
for the mutating stuff and just have var('x')
and function('f')
for the normal stuff? That ship has probably sailed already.
comment:24 in reply to: ↑ 23 Changed 7 years ago by
Replying to nbruin:
Can we have
declare_var('x,y')
anddeclare_function('f')
for the mutating stuff and just havevar('x')
andfunction('f')
for the normal stuff? That ship has probably sailed already.
That would be awesome! Much more consistent AND flexible. +1 from me.
comment:25 in reply to: ↑ 23 Changed 7 years ago by
Replying to nbruin:
I think that's another issue. The problem there is that global namespace
var
andfunction
return a value as well as insert something into the namespace. That's counter to python custom (routines that mutate state normally returnNone
. CompareL.sort()
andsorted(L)
. There are exceptions:L.pop()
).Initially it seems pedantic to enforce such rules in a computer algebra system as well, but the many problems it causes suggests it's not. Can we have
declare_var('x,y')
anddeclare_function('f')
for the mutating stuff and just havevar('x')
andfunction('f')
for the normal stuff? That ship has probably sailed already.
See #17958 for declare_var
and no, this is still a good idea.
comment:26 in reply to: ↑ 3 Changed 7 years ago by
Replying to nbruin:
I suspect that Zimmerman's calculus book uses this stuff quite extensively, so deprecating/changing the current behaviour will likely lead to pushback from him, and probably rightly so.
I'm not sure about function()
and have little time to check now, but regarding var()
, I think we tried to always use x = SR.var('x')
.
And IMO, the version of var
that injects a variable in the global namespace should simply be deprecated, or possibly moved to sage.ext.inteactive_constructors_c
if someone really care about it.
comment:27 Changed 6 years ago by
 Milestone changed from sage6.5 to sage6.6
 Status changed from needs_work to needs_review
So, comment:17 gave part of a review. Can someone please complete it?
comment:28 Changed 6 years ago by
 Status changed from needs_review to needs_work
sage t long src/sage/symbolic/expression.pyx # 1 doctest failed sage t long src/sage/tests/french_book/calculus_doctest.py # 1 doctest failed sage t long src/doc/pt/tutorial/tour_algebra.rst # 1 doctest failed sage t long src/sage/combinat/integer_vector.py # 1 doctest failed
comment:29 Changed 6 years ago by
 Commit changed from 4585e74063e81a8c332a500d016e837069edf8eb to bdc154391e7ce6ceca1226f341ab62579bc3f4ee
comment:30 Changed 6 years ago by
 Commit changed from bdc154391e7ce6ceca1226f341ab62579bc3f4ee to 7c029f3c98032a985c2ca0329dfe940271e9b86b
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
7c029f3  trac 17447: further doctest adjustments

comment:31 Changed 6 years ago by
 Status changed from needs_work to needs_review
comment:32 Changed 6 years ago by
 Milestone changed from sage6.6 to sage6.10
 Reviewers changed from Ralf Stephan to Ralf Stephan, Nils Bruin
 Status changed from needs_review to positive_review
OK, comments #17 and #27 provide a positive review of the first bit. I give a largely positive review to Ralf's doctest adjustments. There was one overcorrection in french_book/calculus_doctest.py
that I corrected. The other changes were just parallel corrections to different translations of one document that have been introduced since the last time this ticket was updated.
patchbot is happy and I think all code is positively reviewed now. Ready for merge!
comment:33 Changed 6 years ago by
 Branch changed from public/17447 to 7c029f3c98032a985c2ca0329dfe940271e9b86b
 Resolution set to fixed
 Status changed from positive_review to closed
For the question about creation and overwriting:
The toplevel
function
has the sideeffect of inserting a binding in the global scope, just as the toplevelvar
does.The library versions of these routines don't do that:
The intended use of the toplevel
function
is not to be assigned but to be called as a statement, just asvar
. There is plenty of documentation in sage that was written by people unaware of this fact (or possibly they think they're being helpful by providing code that doesn't return an error when used with the library versions of these functions)As a result
x=var('x')
andfx=function('f',x)
are quite ubiquitous in the docs. The former is fairly inocuous but the second, as you experience, has a rather nasty side effect. We're violating python's convention here: routines with side effects should return None.I think this is the point where people usually give up fixing this mess (I have). Hopefully you will now follow through.
Note that we can probably not do more than document the issues, which is not going to fix much, because people won't read the doc, but then at least you can point them somewhere when they complain.
I suspect that Zimmerman's calculus book uses this stuff quite extensively, so deprecating/changing the current behaviour will likely lead to pushback from him, and probably rightly so.
(this is the kind of issue that earns sage the reputation of "implement now, design later")