Fix memory policy warning in io.fits#14168
Conversation
mhvk
left a comment
mhvk
left a comment
There was a problem hiding this comment.
This looks good. My tendency would be to include your explanation above in the comment you already included - I think anybody else (or indeed your future self) looking at the code will thank you!
|
Hopefully future self will have a look at the commit message ;) (5fb0787). I also added a link to the Numpy docs in the comment 👍 |
|
I fear astropy is so inconsistent with its commit messages that I at least have never looked at any -- though I do go to relevant github issues if those are linked. Anyway, in the code it will be seen for sure! Happy to have this merged, but I don't think I am allowed to before the tests have passed... |
mhvk
added
the
zzz 💤 merge-when-ci-passes
|
I think there is something wrong with |
Fix astropy#12324 : Fix the warnings emitted by Numpy when transferring ownership by setting ``NPY_ARRAY_OWNDATA``: RuntimeWarning: Trying to dealloc data, but a memory policy is not set. If you take ownership of the data, you must set a base owning the data (e.g. a PyCapsule). The warning was added in [1], and is now visible only when setting the NUMPY_WARN_IF_NO_MEM_POLICY environment variable [2]. For more details and a fix see [3]. [1] numpy/numpy#17582 [2] numpy/numpy#20200 [3] https://github.com/numpy/numpy/blob/main/doc/source/reference/c-api/data_memory.rst#what-happens-when-deallocating-if-there-is-no-policy-set
saimn
force-pushed
the
fits-memory-policy
branch
from
c9daf9a to
32a80a1
Compare
|
Thanks! Let's get this in... |
|
No need to backport this? |
Nothing wrong per se: "ci skip" and "required" are from two different things that sometimes do not play well together. "ci skip" asks CI not to run. "required" is from repo branch protection to make sure important CI jobs are passing before a maintainer is allowed to merge. This was discussed during infrastructure tag-up a while back. We decided the benefit of requiring a bunch of jobs to pass outweighs the inconvenience of "ci skip" requiring an admin to overwrite. |
|
Probably does not need to be backported, the warning is hidden in current Numpy versions. |
3 participants
Fix #12324 : Fix the warnings emitted by Numpy when transferring
ownership by setting
NPY_ARRAY_OWNDATA:The warning was added in [1], and is now visible only when setting the
NUMPY_WARN_IF_NO_MEM_POLICY environment variable [2]. For more details
and a fix see [3].
[1] numpy/numpy#17582
[2] numpy/numpy#20200
[3] https://github.com/numpy/numpy/blob/main/doc/source/reference/c-api/data_memory.rst#what-happens-when-deallocating-if-there-is-no-policy-set