AFM-SPM / TopoStats

An AFM image analysis program to batch process data and obtain statistics from images
https://afm-spm.github.io/TopoStats/
GNU Lesser General Public License v3.0
60 stars 11 forks source link

style(tracingfuncs): Numpydoc validation, PEP8 & tidying #948

Closed ns-rse closed 1 month ago

ns-rse commented 1 month ago

Resolves #920

This commit is a squash of seven individual commits that address Numpydoc validation

Removes unused functions/methods...

Renames methods to adhere to PEP8

Questions

check_vectors_candidate_points - part 1

This function (nee checkVectorsCandidatePoints) takes the parameters x and y but these are never used directly, rather x and y are obtained anew in each iteration of this loop...

        for x_n, y_n in candidate_points:
            x = x_n - x_ref_2
            y = y_n - y_ref_2

            theta = math.atan2(x, y)

            x_y_theta.append([x_n, y_n, abs(theta - ref_theta)])

We should be able to just drop x and y as parameters to this method but I wanted to check as there aren't any tests for these methods/functions.

check_vectors_candidate_points - part 2

I also noticed that in some places check_vectors_candidate_points (nee checkVectorsCandidatePoints) was commented out (lines 83 and 187) in favour of using find_next_best_point (nee findNextBestPoint) but check_vectors_candidate_points is still used in some places.

MaxGamill-Sheffield commented 1 month ago

@ns-rse I have a rough idea what this piece of legacy code does but not how different things could be interchanged. I'd say leave this for now and get this adhering to the standards. And possibly make a new issue to swap these out and see if any tests are affected?

ns-rse commented 1 month ago

Looks good, just if you want to refactor the legacy code or make a new issue?

I've gone with somewhere in-between and addressed Part 1 as removing the unused parameters had no impact on the tests completing successfully and kicked the can done the road for Part 2 and written it up in #953