On Sat, 2022-09-03 at 00:03 +0000, lpsm...@uw.edu wrote:
> Sebastian Berg wrote:
> > On Fri, 2022-08-19 at 23:56 +0000, lpsm...@uw.edu wrote:
> > > Thanks for the information!  I've had to work on other project in
> > > the
> > > meantime, but was able to get back to this again.
> > > In an effort to wrap my head around the project's code, I
> > > realized
> > > that I did not have a line like:
> > > #define NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION
> > > in it.  So, I added the line, fixed the errors the resulted, and
> > > recompiled.  And immediately got segmentation faults.
> 
> > With it, the fields are hidden completely which is the intention. 
> > But
> > that also means the size is wrong for subclassing.  You would have
> > to
> > use `PyArrayObject_fields` although that basically circumvents the
> > deprecation, it somehwat makes sense, you should just only use it
> > in
> > that one place I guess (not for actual access to strides).
> > Overall, I am not sure if this will ever help us much, but the
> > solution
> > seems simple here.  There should be no fundamental changes with the
> > exception of the size of `PyArrayObject_fields`.
> 
> That does clear up some things, but it also confuses me in other
> ways.  The fields in question are fields I've added myself as part of
> the subclass.  Are you saying that if I add new fields, those fields
> are hidden completely?  I don't see how I could interact with them at
> all if that's the case, so I must be misunderstanding something. 
> Among other things, the printing routines need access to them so they
> can print labels in addition to the values.  So it's not just
> 'checking the size' that's the issue; I also need to be able to set,
> modify, and read out their values.  The core fields could remain
> hidden (accessible through the normal routines for the class) but the
> new fields surely wouldn't be?
> 


That is exactly how it is.  You have the following:

    struct MyArray {
        /* Not PyArrayObject with "new" API: */
        PyArrayObject_fields reserved_for_numpy,

        void *myfield1,
        int myfield2,
    };

In that setup, you should actually never access `reserved_for_numpy`. 
You should rather simply cast `MyArray` to `PyArrayObject` if/when
necessary (or just `PyObject`).
If you need to access certain fields like the shape, you would use
`PyArray_DIMS` rather than accessing `->dims` directly.

The reason is that in theory at least we want to be able change what is
inside `PyArrayObject_fields`.  It should be considered opaque!


Unfortunately, you still need the size of `PyArrayObject_fields` even
if you never access it (to define your struct/class).

Now the problem is, NumPy may change the size of `PyArrayObject_fields`
to allow significant improvements for the vast majority of users who do
not subclass in C.

One solution when this happens is to recompile.  But it is not future-
proof (not compatible with future versions of NumPy unless you
recompile for it).
The future proof version would be to add code something like:

    struct numpy_space {
        PyArrayObject_fields reserved_for_numpy,
        void *future_space_reserved_for_numpy[2],
    }

    struct MyArray {
        numpy_space reserved_for_numpy,

        void *myfield1,
        int myfield2,
    };

Which just adds a bit of unused padding space NumPy could use in the
future.  Now, in principle of course that wastes a bit of space...

Also NumPy could theoretically grow beyond the size of `numpy_space`. 
So a solution might be to add a check somewhere during init:

    if (PyArrayType.tp_basicsize > sizeof(numpy_space)) {
        PyErr_SetString(PyExc_RuntimeError,
            "NumPy extended its struct a recompilation "
            "seems necessary...");
        return NULL;
    }

In principle, there are solutions to do this more dynamically (checking
`tp_basicsize` at runtime), but I am not sure it is practical or even
works well in C++.  You would need to store the size as an offset and
always use that offset to access all of your custom fields.

I hope that clarifies things.

Cheers,

Sebastian


> -Lucian
> _______________________________________________
> NumPy-Discussion mailing list -- numpy-discussion@python.org
> To unsubscribe send an email to numpy-discussion-le...@python.org
> https://mail.python.org/mailman3/lists/numpy-discussion.python.org/
> Member address: sebast...@sipsolutions.net


_______________________________________________
NumPy-Discussion mailing list -- numpy-discussion@python.org
To unsubscribe send an email to numpy-discussion-le...@python.org
https://mail.python.org/mailman3/lists/numpy-discussion.python.org/
Member address: arch...@mail-archive.com

Reply via email to