Added assumptions in Dynamicsymbols #18061

Smit-create · 2019-12-17T11:43:09Z

Added assumptions to dynamicsymbols

References to other Issues or PRs

Fixes #17739

Brief description of what is fixed or changed

Added assumptions in dynamicsymbols so it can be positive, real, etc.

Other comments

Release Notes

physics.vector
- Added assumptions to dynamicsymbols
- Added test for dynamicsymbols

sympy-bot · 2019-12-17T11:43:11Z

✅

Hi, I am the SymPy bot (v149). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

physics.vector
- Added assumptions to dynamicsymbols (#18061 by @Smit-create)
- Added test for dynamicsymbols (#18061 by @Smit-create)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.6.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it.

Click here to see the pull request description that was parsed.

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->
Added assumptions to dynamicsymbols
#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234". See
https://github.com/blog/1506-closing-issues-via-pull-requests . Please also
write a comment on that issue linking back to this pull request once it is
open. -->
Fixes#17739

#### Brief description of what is fixed or changed
Added assumptions in dynamicsymbols so it can be positive, real, etc.

#### Other comments


#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* physics.vector
   * Added assumptions to dynamicsymbols
   * Added test for dynamicsymbols
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

moorepants · 2019-12-17T15:04:21Z

@Smit-create Thanks for this PR. Can you add a unit test?

moorepants · 2019-12-17T15:04:48Z

You also need to add release notes for this.

Smit-create · 2019-12-17T15:40:59Z

@moorepants Yes I will add both the things.

moorepants · 2019-12-17T15:45:58Z

On last note: Please add some documentation in the docstring showing how to add assumptions.

Smit-create · 2019-12-17T15:48:26Z

@moorepants Surely, I will add that too.

codecov · 2019-12-17T16:59:39Z

Codecov Report

Merging #18061 into master will increase coverage by 0.029%.
The diff coverage is 100%.

@@              Coverage Diff              @@
##            master    #18061       +/-   ##
=============================================
+ Coverage   74.864%   74.894%   +0.029%     
=============================================
  Files          640       641        +1     
  Lines       166548    166564       +16     
  Branches     39176     39179        +3     
=============================================
+ Hits        124686    124747       +61     
+ Misses       36343     36298       -45     
  Partials      5519      5519

moorepants · 2019-12-17T17:31:01Z

sympy/physics/vector/functions.py

+    >>> q3= dynamicsymbols('q3', positive=True)
+    q3(t)
+    >>> q3.is_positive
+    True


This is good. Also add a note in the Parameters section explaining what **asumptions is. You can reference the symbols() or Symbol() documentation.

moorepants · 2019-12-17T17:31:51Z

sympy/physics/vector/tests/test_functions.py

+    f3=dynamicsymbols('f3',positive=True)
+    assert f1.is_real==None
+    assert f2.is_real
+    assert f3.is_positive


Add spaces around the equals and after commas, as per PEP8.

moorepants · 2019-12-17T17:32:05Z

sympy/physics/vector/tests/test_functions.py

+    f1=dynamicsymbols('f1')
+    f2=dynamicsymbols('f2',real=True)
+    f3=dynamicsymbols('f3',positive=True)
+    assert f1.is_real==None


Should be is None.

Smit-create · 2019-12-17T17:33:03Z

@moorepants Surely I will do the requires changes. Thanks for reviewing.

moorepants · 2019-12-17T17:33:20Z

In the release notes, only add one bullet "added assumptions to dynamicsymbols" and the header should be "physics" not "physics.vector" (I think).

Smit-create · 2019-12-17T17:34:45Z

In the release notes, only add one bullet "added assumptions to dynamicsymbols" and the header should be "physics" not "physics.vector" (I think).

I metioned it physics at first, but it showed be error. So, I changed it to physics.vector

moorepants · 2019-12-17T17:50:30Z

I metioned it physics at first, but it showed be error. So, I changed it to physics.vector

Ok, I thought the comment should have a green checkmark, so I was confused. it is fine.

moorepants · 2019-12-17T17:50:51Z

sympy/physics/vector/functions.py

@@ -587,20 +587,33 @@ def dynamicsymbols(names, level=0):
    level : int
        Level of differentiation of the returned function; d/dt once of t,
        twice of t, etc.
+    Assumptions :


moorepants · 2019-12-17T17:52:25Z

sympy/physics/vector/functions.py

@@ -587,20 +587,33 @@ def dynamicsymbols(names, level=0):
    level : int
        Level of differentiation of the returned function; d/dt once of t,
        twice of t, etc.
+    Assumptions :
+        real = False
+        positive = False


This should say something like "Key-value pairs of assumptions, see symbols for more information". You should read this: https://docs.sympy.org/dev/documentation-style-guide.html

Actually I have referred it from class Symbol, and it had similar kind to explanation of assumption

Well, that sounds like Symbol's docstring is poor then and we should make it better here.

"""Uses symbols and Function for functions of time. Creates a SymPy UndefinedFunction, which is then initialized as a function of a variable, the default being Symbol('t'). Parameters ========== names : str Names of the dynamic symbols you want to create; works the same way as inputs to symbols level : int Level of differentiation of the returned function; d/dt once of t, twice of t, etc. assumptions : real(bool) : This is used to set the dynamicsymbol as real, by default is False positive(bool) : This is used to set the dynamicsymbol as positive, by default is False commutative(bool) : This is used to set the commutative property of a dynamicsymbol, by default is True integer(bool) : This is used to set the dynamicsymbol as integer , by default is False Examples ======== >>> from sympy.physics.vector import dynamicsymbols >>> from sympy import diff, Symbol, sqrt >>> q1 = dynamicsymbols('q1') >>> q1 q1(t) >>> q2 = dynamicsymbols('q2', real=True) >>> q2.is_real True >>> q3 = dynamicsymbols('q3', positive=True) >>> q3.is_positive True >>> q4, q5 = dynamicsymbols('q4,q5', commutative=False) >>> bool(q4*q5 != q5*q4) True >>> q6 = dynamicsymbols('q6', integer=True) >>> q6.is_integer True >>> diff(q1, Symbol('t')) Derivative(q1(t), t) """

Is this the correct way to document?

moorepants · 2019-12-18T14:15:21Z

sympy/physics/vector/functions.py


    Examples
    ========

    >>> from sympy.physics.vector import dynamicsymbols
-    >>> from sympy import diff, Symbol
+    >>> from sympy import diff, Symbol, sqrt


What is the sqrt for?

moorepants · 2019-12-18T14:16:21Z

Remove the sqrt and I think this is good to go after that. Thanks for the contribution.

Smit-create · 2019-12-18T17:06:12Z

Remove the sqrt and I think this is good to go after that. Thanks for the contribution.

Yes, it was completely out of my notice

Smit-create · 2019-12-18T17:12:20Z

Remove the sqrt and I think this is good to go after that. Thanks for the contribution.

Thanks for the help provided, as it was my first contribution I made some silly mistakes but will surely not repeat next time. Thanks for your guidance

moorepants · 2019-12-18T17:50:22Z

sympy/physics/vector/functions.py

+        real(bool) : This is used to set the dynamicsymbol as real, by default is False
+        positive(bool) : This is used to set the dynamicsymbol as positive, by default is False
+        commutative(bool) : This is used to set the commutative property of a dynamicsymbol, by default is True
+        integer(bool) : This is used to set the dynamicsymbol as integer , by default is False


This may not render correctly when converted to HTML for the documentation. I think it will all be treated as one line. You may need to use a list like:

- thing 1 - thing 2

Also wrap the lines at 79 characters.

* real(bool) : This is used to set the dynamicsymbol as real, by default is False * positive(bool) : This is used to set the dynamicsymbol as positive, by default is False * commutative(bool) : This is used to set the commutative property of a dynamicsymbol, by default is True * integer(bool) : This is used to set the dynamicsymbol as integer , by default is False

You mean this way should I write?

Yes, but I think you should check that it renders correctly by building the docs locally.

Yes, I have made the required changes and wrapped the lines to 79 characters

moorepants · 2019-12-18T17:51:01Z

One more likely issue. You can build the docs locally to check if it renders correctly.

Smit-create · 2019-12-19T05:29:25Z

@moorepants I am making the required changes in physics.vector.functions.py but it is showing error in sympy.utilities.lambdify.lambdify though I haven't made any changes there.

Smit-create · 2019-12-19T05:31:42Z

@moorepants I am making the required changes in physics.vector.functions.py but it is showing error in sympy.utilities.lambdify.lambdify though I haven't made any changes there.

________________________________________________________________________________
______________________ sympy.utilities.lambdify.lambdify _______________________
File "/home/travis/miniconda/envs/test-environment/lib/python3.6/site-packages/sympy-1.6.dev0-py3.6.egg/sympy/utilities/lambdify.py", line 402, in sympy.utilities.lambdify.lambdify
Failed example:
    f.__globals__['sin']
Expected:
    <ufunc 'sin'>
Got:
    <function _deprecated.<locals>.wrap.<locals>.call at 0x7fdecc2ec9d8>
**********************************************************************
File "/home/travis/miniconda/envs/test-environment/lib/python3.6/site-packages/sympy-1.6.dev0-py3.6.egg/sympy/utilities/lambdify.py", line 404, in sympy.utilities.lambdify.lambdify
Failed example:
    f.__globals__['cos']
Expected:
    <ufunc 'cos'>
Got:
    <function _deprecated.<locals>.wrap.<locals>.call at 0x7fdecc361510>
**********************************************************************
File "/home/travis/miniconda/envs/test-environment/lib/python3.6/site-packages/sympy-1.6.dev0-py3.6.egg/sympy/utilities/lambdify.py", line 406, in sympy.utilities.lambdify.lambdify
Failed example:
    f.__globals__['sin'] is numpy.sin
Expected:
    True
Got:
    False
===== tests finished: 3689 passed, 1 failed, 2 skipped, in 278.00 seconds ======

moorepants · 2019-12-19T14:02:24Z

There are unrelated test failures.

Smit-create · 2019-12-19T15:27:34Z

Then what I did is correct?

moorepants · 2019-12-19T15:28:43Z

Yes, I think it is good.

Smit-create · 2019-12-19T18:17:32Z

Yes, I think it is good.

So will it get merged?

moorepants · 2019-12-19T18:37:09Z

Once the tests are fixed it will.

Smit-create · 2019-12-19T18:39:26Z

Yes, I think it is good.

Once the tests are fixed it will.

But they are failing unrelated test. So do I need to do anything for fixing them?

moorepants · 2019-12-19T20:39:51Z

But they are failing unrelated test. So do I need to do anything for fixing them?

Wait on #18080 to get merged and then you'll need to merge in the master branch here.

Smit-create · 2019-12-20T14:58:43Z

Is it good to go now?

moorepants · 2019-12-20T15:22:14Z

Yes! Thank you for the contribution.

Added assumptions in Dynamicsymbols

492eeae

moorepants added the physics.mechanics label Dec 17, 2019

Added assumptions and docstrings

ffa9735

Smit-create added 2 commits December 17, 2019 22:33

Added tests for dynamicsymbols

3c8880a

Added test of dynamicsymbols

03710fa

moorepants reviewed Dec 17, 2019

View reviewed changes

Smit-create added 2 commits December 17, 2019 23:07

Added test for dynamicsymbols

252aa75

Added docstrigs to dynamicsymbols

2439e1a

moorepants reviewed Dec 17, 2019

View reviewed changes

Smit-create added 3 commits December 17, 2019 23:53

Added docstrings

59d7fd3

Updated and added docstrings

ef588f8

Added test for dynamicsymbols

9c1cdf1

moorepants reviewed Dec 18, 2019

View reviewed changes

Updated and added docstrings to dynamicsymbols

4bcab50

moorepants reviewed Dec 18, 2019

View reviewed changes

Smit-create added 3 commits December 19, 2019 00:02

Updated Docstrings

b6dabdf

Updates docstrings

aee7f7a

Updated docstrings

efe133d

oscarbenjamin closed this Dec 20, 2019

oscarbenjamin reopened this Dec 20, 2019

moorepants merged commit fbf5089 into sympy:master Dec 20, 2019

Added assumptions in Dynamicsymbols #18061

Added assumptions in Dynamicsymbols #18061

Conversation

Smit-create commented Dec 17, 2019 • edited by moorepants

References to other Issues or PRs

Brief description of what is fixed or changed

Other comments

Release Notes

sympy-bot commented Dec 17, 2019 • edited

moorepants commented Dec 17, 2019

moorepants commented Dec 17, 2019

Smit-create commented Dec 17, 2019

moorepants commented Dec 17, 2019

Smit-create commented Dec 17, 2019

codecov bot commented Dec 17, 2019 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Smit-create commented Dec 17, 2019

moorepants commented Dec 17, 2019

Smit-create commented Dec 17, 2019

moorepants commented Dec 17, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Smit-create Dec 18, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moorepants commented Dec 18, 2019

Smit-create commented Dec 18, 2019

Smit-create commented Dec 18, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moorepants commented Dec 18, 2019

Smit-create commented Dec 19, 2019

Smit-create commented Dec 19, 2019

moorepants commented Dec 19, 2019

Smit-create commented Dec 19, 2019

moorepants commented Dec 19, 2019

Smit-create commented Dec 19, 2019

moorepants commented Dec 19, 2019

Smit-create commented Dec 19, 2019

moorepants commented Dec 19, 2019

Smit-create commented Dec 20, 2019

moorepants commented Dec 20, 2019

Smit-create commented Dec 17, 2019 •

edited by moorepants

sympy-bot commented Dec 17, 2019 •

edited

codecov bot commented Dec 17, 2019 •

edited

Smit-create Dec 18, 2019 •

edited