Return TWave on multiplication with scalar. #8895
Conversation
|
Why a new PR? |
|
Had a bad rebase. |
It's ok. You should have asked on the same PR. Let's wait for the tests to pass. |
|
Next time I will keep that in mind. |
|
Did it fail because the docstring example comes before the Error log : File "/home/travis/virtualenv/python2.6.9/lib/python2.6/site-packages/sympy-0.7.6_git-py2.6.egg/sympy/physics/optics/waves.py", line 75, in sympy.physics.optics.waves.TWave
Failed example:
w4.amplitude
Exception raised:
Traceback (most recent call last):
File "/opt/python/2.6.9/lib/python2.6/doctest.py", line 1253, in __run
compileflags, 1) in test.globs
File "<doctest sympy.physics.optics.waves.TWave[12]>", line 1, in <module>
w4.amplitude
AttributeError: 'Mul' object has no attribute 'amplitude' |
|
I think it failed because Can I make changes to this PR? |
|
@kevinventullo Maybe you are right. Implemented |
|
@debugger22 Checks passed. Please review. |
|
Please squash all your commits into one. |
| @@ -246,6 +251,12 @@ def __str__(self): | |||
|
|
|||
| __repr__ = __str__ | |||
|
|
|||
| def __mul__(self, float): | |||
| return TWave(self._amplitude*float, self._frequency, self._phase) | |||
There was a problem hiding this comment.
IMO the fix here (#8908) is better. You are not checking for the type of your variable float.
There was a problem hiding this comment.
The name of the variable doesn't matter. You should always name the second argument to these functions other. Definitely don't name it float, which overwrites a builtin.
| @@ -24,7 +24,7 @@ class TWave(Expr): | |||
| they can be changed later with respective methods provided. | |||
|
|
|||
| It has been represented as :math:`A \times cos(k*x - \omega \times t + \phi )` | |||
| where :math:`A` is amplitude, :math:`\omega` is angular velocity, :math:`k`is | |||
| where :math:`A` is amplitude, :math:`\omega` is angular frequency, :math:`k`is | |||
There was a problem hiding this comment.
This change is not in the other PR.
There was a problem hiding this comment.
Asked the author of that PR to make the change.
There was a problem hiding this comment.
Ok I added that change to my PR.
|
@darshanime Would you still like to work on this PR? It looks like a good addition. |
Same as #8888
Sorry for the inconvenience.
On trying to multiply
TWaveobject fromsympy.physics.opticsmodule with a scalar,sympy.core.mul.Mulwas returned.PR to return
TWavewith amplitude scaled.