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

Move calculation from Del class to gradient, curl, divergence. #12749

Merged
merged 10 commits into from Jun 18, 2017

Conversation

Projects
None yet
4 participants
@szymag
Contributor

szymag commented Jun 15, 2017

Now Delop is only symbol, independent from coordinate system.

TODO:

  • add warning in Del class, because this PR introduce changes in constructor.
  • resolve problem with some tests which are now disabled
  • Update comment. In Del operators are described calculations which are not there
Show outdated Hide outdated sympy/vector/coordsysrect.py Outdated
Show outdated Hide outdated sympy/vector/deloperator.py Outdated
Show outdated Hide outdated sympy/vector/tests/test_field_functions.py Outdated
Move calculation from Del class to gradient, curl, divergence.
Now Delop is only symbol, independent from coordinate system.
Show outdated Hide outdated sympy/vector/vector.py Outdated
Show outdated Hide outdated sympy/vector/deloperator.py Outdated
@moorepants

This comment has been minimized.

Show comment
Hide comment
@moorepants

moorepants Jun 15, 2017

Member

It seems to me that this is a backwards incompatible change. Make sure to follow our deprecation policy: https://github.com/sympy/sympy/wiki/Deprecating-policy

Member

moorepants commented Jun 15, 2017

It seems to me that this is a backwards incompatible change. Make sure to follow our deprecation policy: https://github.com/sympy/sympy/wiki/Deprecating-policy

szymag added some commits Jun 16, 2017

Add warning in CoordSysCartesian.
Using delop method in CoordSysCartesian cause warning.
feature="delop operator inside coordinate system",
useinstead="it as instance Del class",
deprecated_since_version="1.1"
).warn()

This comment has been minimized.

@szymag

szymag Jun 16, 2017

Contributor

Here I add warning, so using delop via CoordSysCartesian cause warning

@szymag

szymag Jun 16, 2017

Contributor

Here I add warning, so using delop via CoordSysCartesian cause warning

This comment has been minimized.

@Upabjojr

Upabjojr Jun 17, 2017

Contributor

OK

@Upabjojr

Upabjojr Jun 17, 2017

Contributor

OK

@szymag

This comment has been minimized.

Show comment
Hide comment
@szymag

szymag Jun 17, 2017

Contributor

ping @Upabjojr
I need to add try statement in _get_coord_sys_from_expr because symbol or Python int don't have attribute atoms. After that I was able to handle this type of argument in directional_derivative in Vector.

Contributor

szymag commented Jun 17, 2017

ping @Upabjojr
I need to add try statement in _get_coord_sys_from_expr because symbol or Python int don't have attribute atoms. After that I was able to handle this type of argument in directional_derivative in Vector.

@Upabjojr

This comment has been minimized.

Show comment
Hide comment
@Upabjojr

Upabjojr Jun 17, 2017

Contributor

I need to add try statement in _get_coord_sys_from_expr because symbol or Python int don't have attribute atoms.

That means there is a bug somewhere. We should make sure that all elements in the expression tree are SymPy integers. (Just not in this PR!)

Contributor

Upabjojr commented Jun 17, 2017

I need to add try statement in _get_coord_sys_from_expr because symbol or Python int don't have attribute atoms.

That means there is a bug somewhere. We should make sure that all elements in the expression tree are SymPy integers. (Just not in this PR!)

Show outdated Hide outdated sympy/vector/deloperator.py Outdated
Show outdated Hide outdated sympy/vector/deloperator.py Outdated
try:
coord_sys = list(expr.atoms(CoordSysCartesian))
if len(coord_sys) == 1:

This comment has been minimized.

@Upabjojr

Upabjojr Jun 17, 2017

Contributor

In the future we will have to deal with mixed coordinate systems. But it's OK for now, let's keep it like this temporarily.

@Upabjojr

Upabjojr Jun 17, 2017

Contributor

In the future we will have to deal with mixed coordinate systems. But it's OK for now, let's keep it like this temporarily.

@@ -6,7 +6,6 @@
from sympy.vector.deloperator import Del
from sympy.vector.coordsysrect import CoordSysCartesian
from sympy.vector.functions import (express, matrix_to_vector,
curl, divergence, gradient,

This comment has been minimized.

@Upabjojr

Upabjojr Jun 17, 2017

Contributor

why this?

@Upabjojr

Upabjojr Jun 17, 2017

Contributor

why this?

This comment has been minimized.

@szymag

szymag Jun 17, 2017

Contributor

We are importing these functions from operators.py

@szymag

szymag Jun 17, 2017

Contributor

We are importing these functions from operators.py

Show outdated Hide outdated sympy/vector/coordsysrect.py Outdated
feature="delop operator inside coordinate system",
useinstead="it as instance Del class",
deprecated_since_version="1.1"
).warn()

This comment has been minimized.

@Upabjojr

Upabjojr Jun 17, 2017

Contributor

OK

@Upabjojr

Upabjojr Jun 17, 2017

Contributor

OK

Show outdated Hide outdated sympy/vector/coordsysrect.py Outdated
Show outdated Hide outdated sympy/vector/deloperator.py Outdated
@szymag

This comment has been minimized.

Show comment
Hide comment
@szymag

szymag Jun 17, 2017

Contributor

ping @Upabjojr
I think that this PR is ready to merge.

Contributor

szymag commented Jun 17, 2017

ping @Upabjojr
I think that this PR is ready to merge.

@Upabjojr

This comment has been minimized.

Show comment
Hide comment
@Upabjojr

Upabjojr Jun 18, 2017

Contributor

One test is still failing because of deprecation.

Contributor

Upabjojr commented Jun 18, 2017

One test is still failing because of deprecation.

@Upabjojr Upabjojr merged commit 97c5d18 into sympy:master Jun 18, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@asmeurer

This comment has been minimized.

Show comment
Hide comment
@asmeurer

asmeurer Jul 2, 2017

Member

@Upabjojr @szymag in the future, please add a deprecation issue for any deprecated behavior. See https://github.com/sympy/sympy/wiki/Deprecating-policy. For now I've opened #12858. Please write there what is deprecated and why.

Member

asmeurer commented Jul 2, 2017

@Upabjojr @szymag in the future, please add a deprecation issue for any deprecated behavior. See https://github.com/sympy/sympy/wiki/Deprecating-policy. For now I've opened #12858. Please write there what is deprecated and why.

@szymag szymag deleted the szymag:del_operator branch Jul 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment