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
Remove function call syntax for symbolic expressions #14270
Comments
Author: Punarbasu Purkayastha |
This comment has been minimized.
This comment has been minimized.
Work Issues: fix doctests |
Changed work issues from fix doctests to fix combinat/tutorial.py |
comment:3
I am able to fix all the doctests except for the following one in combinat/tutorial.py. The example which fails seems to make no sense to me. A
Let us try to "fix" this (
But now the next command in the tutorial gives
Why is all this happening? Looking at the documentation of
And what exactly are we substituting above?
We are substituting a symbolic function with a symbolic expression! How was this even working earlier? What should I do with this portion of the tutorial? Delete this? |
Apply to devel/sage |
comment:4
Attachment: trac_14270-raise_error_on_function-call.patch.gz Replying to @ppurka: > What should I do with this portion of the tutorial? Delete this? I'm cc:ing Nicolas, who should know what is going on here. |
comment:5
I'd also say that I think the prep tutorial one should still talk about this at some length, to explain (in the event this is done) why this doesn't work, because a lot of people will now and forevermore expect that it will work. Similarly, most of these examples presumably should be moved to the new syntax, perhaps even with an explanatory remark as to exactly why that is the syntax. For instance, how would one do the matrix example Rant I don't actually want to rehash any more, but put here for completeness:
|
comment:6
Replying to @kcrisman:
IMHO, the people who expect this to still work need to change their code. It has been in deprecated mode for over four years. That's more than enough time to change their habit and old code. I think someone hasn't complained before either because they are complacent or because they don't use this at all. While teaching a course with Sage, I remember that we ourselves ran into this problem with the students. It was annoying and confusing because we were unaware of the code and how to fix it. We would just ask the students to ignore those warnings. What would a beginner do after defining I agree with you that it should be explained in the tutorial that there is a difference between symbolic functions and symbolic expressions and python functions. |
This comment has been minimized.
This comment has been minimized.
comment:12
This is actually the same as #8214, but that has no code, so I'm copying the two links from there to here before declaring it a dupe. |
comment:13
This ticket is unlikely to get fixed unless someone who knows the code really well addresses comment:3 |
comment:14
Replying to @ppurka:
OK, from what you've written down here, I can come up with one way of rewriting the example.
I think this should be deleted. Use s0 if you want that and just leave C bound to the variable. It's already bad enough that we need "C as a function" further down.
Didn't we need Cf before to arrive at
I think the following should work (it does in vanilla, without a deprecation warning):
I'm not so sure it "works". It doesn't give an error (which is surprising in its own right; perhaps that should change), but I don't think it gives the same answer. In vanilla:
Rightly so, judging from the results above.
I guess because symbolic expressions were deprecated "functions": you could call then with symbolic arguments and get a symbolic expression, at the cost of a deprecation warning.
No, just do
That's probably a useful example to have anyway. It shows how to turn a symbolic expression into a function in a non-ambiguous way, because you need to specify argument order. |
This comment has been minimized.
This comment has been minimized.
comment:44
|
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
|
comment:48
I think this needs a rebase onto rc0? Trac merge failed even though it rebases cleanly. |
Branch pushed to git repo; I updated commit sha1. New commits:
|
Reviewer: Matthias Koeppe, ... |
comment:52
We have given people plenty of time to stop using this. Time to bring down the hammer. I am setting this to a positive review, but setting it to 9.5 just to make sure it does not go into the soon-to-be-released 9.4 to get more testing. |
Changed reviewer from Matthias Koeppe, ... to Matthias Koeppe, Travis Scrimshaw |
comment:53
Thank you! |
Changed branch from u/mkoeppe/remove_function_call_syntax_for_symbolic_expressions to |
The function call syntax for symbolic expressions has been deprecated for about 12 years. It is about time it raised some errors. This will prevent people from getting confused because of behavior like this.
See also the threads
Update in 9.4.beta4: The deprecation warning introduced in #5930 was not issued in direct interactive use since Sage 8.4 (despite being doctested):
(fixed in #32139)
Follow-up: #32227 Deprecate methods
arguments
(alias:args
),number_of_arguments
,_fast_callable_
for non-callable symbolic expressionsCC: @nthiery @kini @DaveWitteMorris @nbruin @egourgoulhon
Component: symbolics
Author: Punarbasu Purkayastha, Matthias Koeppe
Branch/Commit:
9891a51
Reviewer: Matthias Koeppe, Travis Scrimshaw
Issue created by migration from https://trac.sagemath.org/ticket/14270
The text was updated successfully, but these errors were encountered: