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

Constructor fix #13186

Merged
merged 5 commits into from Aug 25, 2017

Conversation

Projects
None yet
2 participants
@szymag
Contributor

szymag commented Aug 24, 2017

Add a method projections in vector class which returns components but include also components which has 0.

Fixes constructor. We cannot just write vector[0] to get first component of this vector so I use already created method to obtain components for translation vector.

@szymag

This comment has been minimized.

Show comment
Hide comment
@szymag

szymag Aug 24, 2017

Contributor

ping @Upabjojr

Contributor

szymag commented Aug 24, 2017

ping @Upabjojr

Show outdated Hide outdated sympy/vector/vector.py
Show outdated Hide outdated sympy/vector/vector.py
@Upabjojr

This comment has been minimized.

Show comment
Hide comment
@Upabjojr

Upabjojr Aug 24, 2017

Contributor

I don't really understand the purpose of this PR.

Contributor

Upabjojr commented Aug 24, 2017

I don't really understand the purpose of this PR.

@szymag

This comment has been minimized.

Show comment
Hide comment
@szymag

szymag Aug 24, 2017

Contributor

The aim of this PR is to fix constructor in CoordSys3D because composition of translation and rotation doesn't work.
I decided to add a method to Vector which makes modification easier

Contributor

szymag commented Aug 24, 2017

The aim of this PR is to fix constructor in CoordSys3D because composition of translation and rotation doesn't work.
I decided to add a method to Vector which makes modification easier

@szymag

This comment has been minimized.

Show comment
Hide comment
@szymag

szymag Aug 24, 2017

Contributor

Without that modification the following code doesn't work:

from sympy.vector import CoordSys3D
C = CoordSys3D('C')

a = C.orient_new_axis('a', q, C.i)
a.transformation_from_parent()

The output is following:

 line 141, in <lambda>   [x-l[0], y-l[1], z-l[2]])
TypeError: 'VectorZero' object does not support indexing

Basically inverse transformation equations were wrong defined

Contributor

szymag commented Aug 24, 2017

Without that modification the following code doesn't work:

from sympy.vector import CoordSys3D
C = CoordSys3D('C')

a = C.orient_new_axis('a', q, C.i)
a.transformation_from_parent()

The output is following:

 line 141, in <lambda>   [x-l[0], y-l[1], z-l[2]])
TypeError: 'VectorZero' object does not support indexing

Basically inverse transformation equations were wrong defined

Show outdated Hide outdated sympy/vector/vector.py
Show outdated Hide outdated sympy/vector/coordsysrect.py
@Upabjojr

This comment has been minimized.

Show comment
Hide comment
@Upabjojr

Upabjojr Aug 24, 2017

Contributor

In components we can't rely on order of returned value because elements are cut when projection on base vector is 0

We can't in this PR as well. You put the elements in a dict, then read the elements out of the dict through .values(). Basically the output order will be random, maybe even dependent on the Python version and operating system, as the dict does not have a clearly defined element order.

Contributor

Upabjojr commented Aug 24, 2017

In components we can't rely on order of returned value because elements are cut when projection on base vector is 0

We can't in this PR as well. You put the elements in a dict, then read the elements out of the dict through .values(). Basically the output order will be random, maybe even dependent on the Python version and operating system, as the dict does not have a clearly defined element order.

@Upabjojr Upabjojr merged commit db8ab11 into sympy:master Aug 25, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment