siavashk / pycpd

Pure Numpy Implementation of the Coherent Point Drift Algorithm
MIT License
513 stars 115 forks source link

Lack of documentation or descriptive coding conventions #24

Closed munsanje closed 5 years ago

munsanje commented 5 years ago

This issue was raised in #12 and then closed, despite the actual issue persisting. Think it would be valuable to leave this open so that potential contributors know that it is an open problem.

Would also be useful if the variable names were descriptive, so that the burden of explicitly documenting the code is lessened. Variables like B and t do nothing to explain the functionality and hinder understanding.

siavashk commented 5 years ago

You are right. I wrote pycpd over the weekend as part of an invited talk and to be honest, I did not think anyone other than the people who attended the talk would be interested.

I tried to have variables mirror their counterparts in the original paper as this package was meant for educational use. So I will probably not rename (B, t) to something like (affine_transformation_matrix, translation_vector). What I will do is point out the connection in comments.

If you have other suggestions on how the package can be improved, you are more than welcome to post them here, or better yet, fork the repository and submit a pull request.

siavashk commented 5 years ago

Have a look at cbd1d25b8f207f8ed3aed28b9a2c7f05c1cb94e4. If this addresses your issue, I will merge with master and close the issue.

siavashk commented 5 years ago

Closing this due to inactivity.