Skip to content
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

deprecate rotation_matrix_* functions #294

Closed
CagtayFabry opened this issue Mar 19, 2021 · 3 comments · Fixed by #317
Closed

deprecate rotation_matrix_* functions #294

CagtayFabry opened this issue Mar 19, 2021 · 3 comments · Fixed by #317
Assignees
Labels
deprecation function or dependency deprecations low priority low priority issues during ☕
Milestone

Comments

@CagtayFabry
Copy link
Member

CagtayFabry commented Mar 19, 2021

I think we can deprecate the transformations.rotation_matrix_* functions in favor of WXRotation.as_matrix to allow more input options and clean up the API a bit

They are mostly used for testing

@CagtayFabry CagtayFabry added deprecation function or dependency deprecations low priority low priority issues during ☕ labels Mar 19, 2021
@marscher
Copy link
Contributor

For the aid of deprecating funtions, at least in my experience, a deprecation class/function decorator is very useful. It shows a warning indicating that the called object is deprecated and can be turned into a an error via the warnings module to clean up all usages within the code base. Users will just see the warning for a couple of versions and can adopt their code accordingly.

Back to topic, I agree that these functions are a indeed a duplication of features and should be replaced by scipy Rotations.

@vhirtham
Copy link
Collaborator

Since we are the only users so far, I think we can just remove it. However, having a deprecated decorator is probably a nice thing for the future. I'll add one...

@vhirtham vhirtham mentioned this issue Mar 22, 2021
4 tasks
@CagtayFabry
Copy link
Member Author

yes this should only "break" internal code so removing should be fine :)
we can redefine these in the tests directly to simplify refactoring

@CagtayFabry CagtayFabry added this to the 0.4.0 milestone Mar 22, 2021
@vhirtham vhirtham self-assigned this Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation function or dependency deprecations low priority low priority issues during ☕
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants