seung-lab / euclidean-distance-transform-3d

Euclidean distance & signed distance transform for multi-label 3D anisotropic images using marching parabolas.
GNU General Public License v3.0
234 stars 37 forks source link

Add memory order conversion to handle non-contiguous input arrays #19

Closed maweigert closed 4 years ago

maweigert commented 4 years ago

This PR fixes current unexpected behaviour for non-contiguous input arrays.

william-silversmith commented 4 years ago

Thank you for the PR! I like what you did here with one exception. It's in theory possible for an array to be in F order with a C layout (e.g. if someone wrote to it as arr[ z, y, x ] or acquires it from a C extension). Maybe it would be better to have the default order be "auto" or "detect" which does what you want here, but allows one to specify "C" or "F" manually if that's the behavior they want?

Would that be too confusing?

Happy new year!

william-silversmith commented 4 years ago

Looks like 'A' is the numpy version of 'auto'. 'K' is pretty similar.

https://docs.scipy.org/doc/numpy/reference/generated/numpy.array.html

Something like this maybe?

  if not data.flags.c_contiguous and not data.flags.f_contiguous:
    order = 'C' if order != 'F' else 'F'
    data = np.copy(data, order=order)
  elif order == 'A' and data.flags.c_contiguous:
    order = 'C'
  elif order == 'A' and data.flags.f_contiguous:
    order = 'F'

  if order not in ('C', 'F'):
    raise ValueError("order must be 'A', 'C' or 'F'. Got: " + str(order))
william-silversmith commented 4 years ago

Thanks for the contribution! I ended up doing it in #22 to allow people to choose how they wish to interpret the array. I'm always happy to get feedback and discuss additional changes. :) Thanks for being the impetus for making dealing with Fortran arrays easy!

maweigert commented 4 years ago

Sure! Btw, we're using edt in https://github.com/mpicbg-csbd/stardist and its so much faster than the scipy.ndimage version! Thanks for the package! :)

william-silversmith commented 4 years ago

Cool! I have a very informal release process, so I'll need to get all my computers together tomorrow to release new binaries.

By the way, I was glancing through your code, and realized I might have another library you might enjoy: https://github.com/seung-lab/fill_voids

fill_voids is a replacement for binary_fill_holes which is MUCH faster and lower memory. Check it out!

maweigert commented 4 years ago

Thanks! I love issues turning into knowledge :)