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

Change class name from CoordSysCartesian to CoordSys3D. #12829

Merged
merged 10 commits into from Jul 2, 2017

Conversation

Projects
None yet
3 participants
@szymag
Contributor

szymag commented Jun 28, 2017

Vector module is preparing to handle different type of curvilinear coordinate system, so main class CoordSysCartesian has misleading name.
This PR changes only strings from CoordSysCartesian to CoordSys3D.

TODO:

  • update documentation
@szymag

This comment has been minimized.

Show comment
Hide comment
@szymag

szymag Jun 28, 2017

Contributor

ping @Upabjojr

Contributor

szymag commented Jun 28, 2017

ping @Upabjojr

C = CoordSysCartesian('C')
C = CoordSys3D('C')

This comment has been minimized.

@moorepants

moorepants Jun 28, 2017

Member

Tests for deprecated functions should be left in place until the deprecated function is removed. This ensures that the deprecated function doesn't break on any future changes to SymPy that occur as long as the deprecated function is in place.

@moorepants

moorepants Jun 28, 2017

Member

Tests for deprecated functions should be left in place until the deprecated function is removed. This ensures that the deprecated function doesn't break on any future changes to SymPy that occur as long as the deprecated function is in place.

This comment has been minimized.

@Upabjojr

Upabjojr Jun 29, 2017

Contributor

I've got the impression that deprecation warnings are treated as errors in the tests.

@Upabjojr

Upabjojr Jun 29, 2017

Contributor

I've got the impression that deprecation warnings are treated as errors in the tests.

This comment has been minimized.

@moorepants

moorepants Jun 29, 2017

Member

You have to filter them so that the travis tests don't error but simply emit the warning.

@moorepants

moorepants Jun 29, 2017

Member

You have to filter them so that the travis tests don't error but simply emit the warning.

This comment has been minimized.

@moorepants

moorepants Jun 29, 2017

Member

There are a number of examples. Use this grep:

grep -rn "category=SymPyDeprecationWarning" sympy --include \*.py
@moorepants

moorepants Jun 29, 2017

Member

There are a number of examples. Use this grep:

grep -rn "category=SymPyDeprecationWarning" sympy --include \*.py

This comment has been minimized.

@Upabjojr

Upabjojr Jun 29, 2017

Contributor

@szymag could you add a test showing that the deprecated version behaves exactly like CoordSys3D? You should use filter warning.

@Upabjojr

Upabjojr Jun 29, 2017

Contributor

@szymag could you add a test showing that the deprecated version behaves exactly like CoordSys3D? You should use filter warning.

This comment has been minimized.

@szymag

szymag Jun 29, 2017

Contributor

OK, I will check how to use filter checking.

@szymag

szymag Jun 29, 2017

Contributor

OK, I will check how to use filter checking.

This comment has been minimized.

@Upabjojr

Upabjojr Jun 29, 2017

Contributor

I think we just need one test to check assert CoordSysCartesian("C") == CoordSys3D("C"), maybe also repeated with some optional arguments.

@Upabjojr

Upabjojr Jun 29, 2017

Contributor

I think we just need one test to check assert CoordSysCartesian("C") == CoordSys3D("C"), maybe also repeated with some optional arguments.

This comment has been minimized.

@moorepants

moorepants Jun 29, 2017

Member

Yes, one is sufficient here.

@moorepants

moorepants Jun 29, 2017

Member

Yes, one is sufficient here.

This comment has been minimized.

@szymag

szymag Jun 29, 2017

Contributor

I think we just need one test to check assert CoordSysCartesian("C") == CoordSys3D("C"), maybe also repeated with some optional arguments.

I have the same idea

@szymag

szymag Jun 29, 2017

Contributor

I think we just need one test to check assert CoordSysCartesian("C") == CoordSys3D("C"), maybe also repeated with some optional arguments.

I have the same idea

@Upabjojr Upabjojr merged commit 1a50e9d into sympy:master Jul 2, 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