Use PyArray_SetBaseObject as ndarray.base is readonly property

[rafalstapinski]

Change 1

Numpy 1.7 introduced the PyArray_SetBaseObject method of setting ndarray.base property as in recent versions base has become a read-only.

When compiling rawpy with a recent version of numpy (1.23.2) we get the following errors:

  Error compiling Cython file:
  ------------------------------------------------------------
  ...
                  raise RuntimeError('unsupported raw data')

              # ndarr must hold a reference to this object,
              # otherwise the underlying data gets lost when the RawPy instance gets out of scope
              # (which would trigger __dealloc__)
              ndarr.base = <PyObject*> self
                   ^
  ------------------------------------------------------------

  rawpy/_rawpy.pyx:509:17: Assignment to a read-only property

  Error compiling Cython file:
  ------------------------------------------------------------
  ...
          shape[2] = <np.npy_intp> self.processed_image.colors
          cdef np.ndarray ndarr
          ndarr = np.PyArray_SimpleNewFromData(3, shape,
                                               np.NPY_UINT8 if self.processed_image.bits == 8 else np.NPY_UINT16,
                                               self.processed_image.data)
          ndarr.base = <PyObject*> self
               ^
  ------------------------------------------------------------

  rawpy/_rawpy.pyx:1191:13: Assignment to a read-only property

Using numpy's PyArray_SetBaseObject fixes this (per: https://github.com/cython/cython/issues/3690#issuecomment-645665793)

Breaking out from this thread: https://github.com/letmaik/rawpy/issues/171#issuecomment-1522950983

Change 2

Additionally the signature for skimage.filters.rank.median has changed, selem has changed to footprint so have updated that as well.

[rafalstapinski]

@letmaik - any thoughts on setting a minimum numpy version for rawpy? Given this method was introduced in numpy==1.7, rawpy coupled with lower versions of numpy will fail.

[letmaik]

@rafalstapinski Setting a lower version bound is theoretically possible via version specifiers in install_requires in setup.py but I wouldn't bother to be honest as noone would try to build rawpy with such an ancient version of numpy, and I don't want to accurately maintain and update this lower bound in the future.

[letmaik]

Looks like something else is currently broken unrelated to your PR which makes CI fail, probably due to an update of scikit-image. Do you want to have a look by any chance? I'm a little busy currently.

[rafalstapinski]

Yeah, I'll take a poke around.

[rafalstapinski]

I don't have the full context of your reasoning behind allowing for trying to use median from skimage or opencv depending on availability. Totally fine with just using the new skimage signature (footprint, instead of selem) as scikit-image seems to have renamed it a few versions ago - but would it be better to just stick to one library (and making it a requirement) instead of trying for both?

[letmaik]

OpenCV, at least back then and maybe still now, is faster than scikit-image but is not as general, see the code comments for details. Let’s keep it as is for now and just patch the signature.

On 27 Apr 2023, at 01:00, Rafal Stapinski @.***> wrote:



I don't have the full context of your reasoning behind allowing for trying to use median from skimage or opencv depending on availability. Totally fine with just using the new skimage signaturehttps://scikit-image.org/docs/stable/api/skimage.filters.rank.html#median (footprint, instead of selem) as scikit-image seems to have renamed it a few versions ago - but would it be better to just stick to one library (and making it a requirement) instead of trying for both?

— Reply to this email directly, view it on GitHubhttps://github.com/letmaik/rawpy/pull/201#issuecomment-1524237807, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAEBULHDJDWPULFBJQPSXLLXDGZJHANCNFSM6AAAAAAXMV7ANI. You are receiving this because you were mentioned.Message ID: @.***>

[rafalstapinski]

Sounds good - I updated the calls, want to run the pipelines again?

[rafalstapinski]

Alrighty - looks like its all passing. Is your preference to break these out into two isolated changes, or just merge as is? Updated the PR message to reflect both changes.

[letmaik]

Thanks again! Will kick off a new release soon.