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

obsolete functions in lib.distances #2072

Closed
zemanj opened this issue Sep 12, 2018 · 2 comments
Closed

obsolete functions in lib.distances #2072

zemanj opened this issue Sep 12, 2018 · 2 comments
Assignees

Comments

@zemanj
Copy link
Member

zemanj commented Sep 12, 2018

Problem description

There are three functions in lib.distances that were introduced in version 0.18.1 but became obsolete since PR #2048:

  • calc_distance()
  • calc_angle()
  • calc_dihedral()

These functions were intended for use with single coordinates (numpy arrays with shape (3,)) but are no longer needed since the functions calc_bonds(), calc_angles(), and calc_dihedrals() now also accept single coordinates as input (and, likewise, return a single distance / angle / dihedral in that case).

To be precise, we now have the following equivalences:

  • calc_distance(...) ↔️ calc_bonds(...)
  • calc_angle(...) ↔️ np.rad2deg(calc_angles(...))
  • calc_dihedral(...) ↔️ np.rad2deg(calc_dihedrals(...))

The obsolete functions are used in the following files:

  • analysis/hbonds/hbond_analysis.py
  • analysis/hbonds/wbridge_analysis.py
  • core/topologyobjects.py

Possible solutions

  1. Leave everything as is (but maybe update the docstrings).
  2. Mark the three functions as deprecated and replace their occurrences in the code according to the equivalent functions as listed above.
  3. Remove the functions and replace them (as in solution 2)

Personally, I'm not in favor of solution 1.
The most important question for me is whether 0.18.1 was an official release, or in other words: Can we choose solution 3 or do we need solution 2?

Current version of MDAnalysis
0.18.1-dev

@richardjgowers
Copy link
Member

18.1 never existed, so we don't have to worry about deprecations.

@zemanj
Copy link
Member Author

zemanj commented Sep 12, 2018

Thx! I'll go ahead and open a PR.

@zemanj zemanj self-assigned this Sep 13, 2018
zemanj added a commit to zemanj/mdanalysis that referenced this issue Sep 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants