python-package-best-practices
python-package-best-practices copied to clipboard
MolSSI-Education
calculate_distance running into exception when build_bond_list tested
I was trying to practice Episode 07 - Python Testing And when I reached the build_bond_list test, invoking the command pytest -v and it throws an Exception:
(molssi_best_practices) radifar->mymolpy$ pytest -v
====================================================== test session starts ======================================================
platform linux -- Python 3.7.7, pytest-5.4.3, py-1.8.2, pluggy-0.13.1 -- /home/radifar/anaconda3/envs/molssi_best_practices/bin/python
cachedir: .pytest_cache
rootdir: /media/radifar/radifar-dsl/Python/molssi-best-practice/mymolpy
plugins: cov-2.10.0
collected 4 items
mymolpy/tests/test_measure.py::test_calculate_distance PASSED [ 25%]
mymolpy/tests/test_measure.py::test_calculate_angle PASSED [ 50%]
mymolpy/tests/test_molecule.py::test_build_bond_list FAILED [ 75%]
mymolpy/tests/test_mymolpy.py::test_mymolpy_imported PASSED [100%]
=========================================================== FAILURES ============================================================
_____________________________________________________ test_build_bond_list ______________________________________________________
def test_build_bond_list():
coordinates = np.array([
[1, 1, 1],
[2.4, 1, 1],
[-0.4, 1, 1],
[1, 1, 2.4],
[1, 1, -0.4]
])
> bonds = mymolpy.build_bond_list(coordinates)
mymolpy/tests/test_molecule.py:18:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
mymolpy/molecule.py:15: in build_bond_list
distance = calculate_distance(coordinates[atom1], coordinates[atom2])
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
rA = array([1., 1., 1.]), rB = array([1., 1., 1.])
def calculate_distance(rA, rB):
"""Calculate the distance between two points.
Parameters
----------
rA, rB : np.ndarray
The coordinates of each point.
Returns
-------
distance : float
The distance between the two points.
Examples
--------
>>> r1 = np.array([0, 0, 0])
>>> r2 = np.array([0, 0.1, 0])
>>> calculate_distance(r1, r2)
0.1
"""
if isinstance(rA, np.ndarray) is False or isinstance(rB, np.ndarray) is False:
raise TypeError("rA and rB must be numpy arrays")
dist_vec = (rA - rB)
distance = np.linalg.norm(dist_vec)
if distance == 0.0:
> raise Exception("Two atoms are located in the same point in space")
E Exception: Two atoms are located in the same point in space
mymolpy/measure.py:35: Exception
==================================================== short test summary info ====================================================
FAILED mymolpy/tests/test_molecule.py::test_build_bond_list - Exception: Two atoms are located in the same point in space
================================================== 1 failed, 3 passed in 0.46s ==================================================
Btw, my package name is mymolpy instead of molecool, and here is my mymolpy/tests/test_molecule.py
"""
Unit and regression test for the molecule module
"""
import mymolpy
import numpy as np
def test_build_bond_list():
coordinates = np.array([
[1, 1, 1],
[2.4, 1, 1],
[-0.4, 1, 1],
[1, 1, 2.4],
[1, 1, -0.4]
])
bonds = mymolpy.build_bond_list(coordinates)
assert len(bonds) == 4
for bond_length in bonds.values():
assert bond_length == 1.4
Here is my mymolpy/molecule.py
"""
Miscelaneous function related to molecule
"""
from .measure import calculate_distance
def build_bond_list(coordinates, max_bond=1.5, min_bond=0):
# Find the bonds in a molecule (set of coordinates) based on distance criteria.
bonds = {}
num_atoms = len(coordinates)
for atom1 in range(num_atoms):
for atom2 in range(atom1, num_atoms):
distance = calculate_distance(coordinates[atom1], coordinates[atom2])
if distance > min_bond and distance < max_bond:
bonds[(atom1, atom2)] = distance
return bonds
And then I tried to debug mymolpy/molecule.py as follows (Notice that I'm adding +1 after atom1 in the inner for loop):
"""
Miscelaneous function related to molecule
"""
from .measure import calculate_distance
def build_bond_list(coordinates, max_bond=1.5, min_bond=0):
# Find the bonds in a molecule (set of coordinates) based on distance criteria.
bonds = {}
num_atoms = len(coordinates)
for atom1 in range(num_atoms):
for atom2 in range(atom1+1, num_atoms):
distance = calculate_distance(coordinates[atom1], coordinates[atom2])
if distance > min_bond and distance < max_bond:
bonds[(atom1, atom2)] = distance
return bonds
And the test run perfectly!
(molssi_best_practices) radifar->mymolpy$ pytest -v
====================================================== test session starts ======================================================
platform linux -- Python 3.7.7, pytest-5.4.3, py-1.8.2, pluggy-0.13.1 -- /home/radifar/anaconda3/envs/molssi_best_practices/bin/python
cachedir: .pytest_cache
rootdir: /media/radifar/radifar-dsl/Python/molssi-best-practice/mymolpy
plugins: cov-2.10.0
collected 4 items
mymolpy/tests/test_measure.py::test_calculate_distance PASSED [ 25%]
mymolpy/tests/test_measure.py::test_calculate_angle PASSED [ 50%]
mymolpy/tests/test_molecule.py::test_build_bond_list PASSED [ 75%]
mymolpy/tests/test_mymolpy.py::test_mymolpy_imported PASSED [100%]
======================================================= 4 passed in 0.36s =======================================================
Good point here. The exception raising was added recently for the last webinar. I am actually in favor of removing this exception from he calculate_distance function because calculate_distance is a general function for distances, and we happen to be using it on atoms.
Yeah me too, I think the calculate_distance function should be for distance calculation only. And the other logic should be handled by the other module in their own specific way (e.g. the bond detection).
In that case, I think the raise Exception when distance == 0 should be cleaned from Episode 4 except the one in Raising Errors section, which is necessary for the sake of demonstration. And then it should also be emphasized that this code is only for demonstration and not to be committed.
The issue is closed, but it remains that actually there is no reason to measure the distance between an atom and itself! This could potentially be changed in the starting material (with the fix suggested by @radifar) . Right now there isn't any "harm" in leaving it, so I'm closing this issue.