search

soft-matter / trackpy

Python particle tracking toolkit
http://soft-matter.github.io/trackpy
Other
445 stars 131 forks source link

Fix for issue #642: refine/brightfield_ring.py _min_edge(), refactor and add `min_percentile` kwarg #654

Closed rbnvrw closed 3 years ago

rbnvrw commented 3 years ago

This commit does a few things:

  1. Adds a min_percentile kwarg to refine/brightfield_ring.py _min_edge(). Before, I just used np.nanmin() there, this was changed to np.nanpercentile(arr, 5) because that worked better for experimental data. In the tests however, the edge is very sharp and therefore it would sometimes fail. With this new kwarg, users can define the threshold based on their use case.
  2. Refactor refine/brightfield_ring.py _min_edge() to get rid of the for loop and only use array operations.
  3. Use np.errstate(invalid='ignore') to allow comparison to np.nan in refine/brightfield_ring.py _min_edge(), these are filtered out later. Also suppress warnings on all NaN slices there, this is OK because those are rows for which a minimum could not be found, these are handled later in the same function.

This fixes the tests that failed in issue #642, that were apparently caused by a different default value for the numpy error state (related to point 3 above).

caspervdw commented 3 years ago

Thanks @rbnvrw. All tests pass, but there is a weird segfault right at the end of the pytest (so after everything passes).

This might have to do with the interpreter closing down, maybe something is deallocated that is already deallocated. I have no clue were to start looking. As this is probably unrelated to this PR, I'll just rerun the tests and see how reproducible this is.

rbnvrw commented 3 years ago

Hmm OK weird.. I have no experience with GitHub actions, so I'm not sure where it could be coming from.. it is only for one (Pip / Python 3.7) so maybe there is something wrong in the environment config there?

caspervdw commented 3 years ago

My first guess would be an incompatibility between specific versions of some compiled packages and/or the python interpreter. But we have many of them, it will be hard to find. Best chance is trying to reproduce the exact environment (so on a linux machine).

Still, this PR seems all right, so, this can be merged.