Revert "fix fdiff to use dummies" #278
Conversation
|
+1 for the revert. |
|
So yeah, there are test failures (I guess I should run the tests before pushing :) Fix coming soon… |
|
OK, so we need to discuss something pending this. See the issue page. |
| @@ -79,6 +79,8 @@ class re(Function): | |||
There was a problem hiding this comment.
The removal of these is OK (they should have been in a separate commit initially). So they should be removed from the revert.
|
+1 to push this in. |
|
This needs to be fixed. I reverted too much. |
|
Also, see the discussion at the issue page. We need to decide on that before anything can be pushed too. |
This reverts commit c20062e. This change was just plain wrong. See issue 1525. Conflicts: sympy/core/function.py
This require derivatives evaluated at a point to really be correct. The previous Derivative(f(g(x)), g(x))*Derivative(g(x), x) was a hack that lead to problems (for example, if you tried to run .doit() on that, it would fail). It was difficult to continue to support this and to not allow erroneous input to diff (like f(x).diff(2)) because of the way the code is implemented with the chain rul. Since the output was technically incorrect anyway, this was removed. Note that this is a workaround (though in my opinion, the old way was too, and it was more hackish). The correct fix would be to implemented derivatives evaluated at a point (issue 1620). I had to wrap some code in classify_ode() around a try, except NotImplementedError, block to make it work with things like f(y/x) in an ode doctest (the classification test was for 1st_exact, but the doctest related to homogeneous_coeff and wasn't exact). Other code might have to do similar things until this gets fixed properly (though no other code in the library code needed to be fixed). See issue 1525 for more information on this issue.
|
This is ready to be reviewed again. See also the discussion (mostly with myself) on the issue page. |
| @@ -575,10 +572,7 @@ class Derivative(Expr): | |||
| Carries out differentiation of the given expression with respect to symbols. | |||
|
|
|||
| expr must define ._eval_derivative(symbol) method that returns | |||
| the differentiation result. This function only needs to consider the | |||
There was a problem hiding this comment.
why the deletions? Is this repeated elsewhere/
There was a problem hiding this comment.
That comes from the reversion commit. Did I revert too much again?
|
OK, this is in. Is there an issue to close? |
|
Yes, 1525. I'll close it. |
This was added in PR sympy#278 to fix a failing test but the test doesn't fail anymore, even without the try/except.
This reverts commit c20062e.
This change was just plain wrong. See issue 1525, starting at comment 3: http://code.google.com/p/sympy/issues/detail?id=1525#c3
Conflicts:
I am still running the tests. If there are any failures, I will fix them and update.
@smichr, you better take a look at this.