BUG, ENH: Access PyArrayMultiIterObject fields using macros. - #24658

Thanks! Sorry, about testing, this is mainly a Cython problem right now. There are some Cython tests in core/test/test_cython.py which use files from core/test/examples/cython. Adding such a test would make sense I think.
Then we can test the attributes access working directly (which will pass after adding the property code).

It is a bit weird to add new macros in a backport for cython, but I think we need to backport this to allow projects using this (for better or worse) to transition to cython 3.

Could you take another look when you can @seberg. I addressed the above comments and added tests as suggested.

Is your code using these constructs public? I wonder if there is a better way to use the MultiIter than to expose these internals.

Is your code using these constructs public? I wonder if there is a better way to use the MultiIter than to expose these internals.

Yes, it is used at several places in this file. An example is this snippet

I thought the addition of macros to access information would be considered better than direct access to the struct fields? Also, is there a reason why the Array Iterator API from here is not part of the public API? I would have preferred that over using PyArray_MultiIter_* since its newer, but it doesn't appear to be accessible via Cython (at least it wasn't at the time I wrote the above code).

I thought the addition of macros to access information would be considered better than direct access to the struct fields?

Macros are indeed better than direct access, but even better for flexibility are getter/setter functions, even if they are statically defined. They at least provide some typing information.

Also, is there a reason ...

I don't think there was ever a demand for it.

I am not sure I understand how we "removed np.broadcast from the 0.29.x cython pxd file in #16986", as the motivation for this PR. All I see is the removal of extern from the declaration. I think this PR should do the minimum to restore the property access via broadcast.numiter and friends for cython 3.0+

I am not sure I understand how we "removed np.broadcast from the 0.29.x cython pxd file in #16986", as the motivation for this PR. All I see is the removal of extern from the declaration. I think this PR should do the minimum to restore the property access via broadcast.numiter and friends for cython 3.0+

In #16986 (comment) I mentioned that the ability to access the fields (either directly or indirectly) of a np.broadcast object is lost in the cython3 init file, compared to the functionality provided by the init file of cython 0.29.X. If you look at https://github.com/scoder/numpy/blob/df769ef591f8a21f8e39ee30c80e09d1b7e0a705/numpy/__init__.cython-30.pxd#L230-L232, you will notice that the fields no longer exist for np.broadcast. That is the motivation for this PR: To restore that functionality by either direct access (via @Property decorator) and/or using the macros/functions defined in https://github.com/numpy/numpy/pull/24658/files/6ab7f384e927fb3d7afea0fa69b2ee88c9d353ee#diff-1e081d27e4d07ed1eaa8fe4af755d81de8c1dc062c919559b9678788dfc691f3 .

You can use the new API and we can add it (not backport). I would have guess that hte few times someone wanted it, they added it to their own file because they didn't want to wait for upstream to have it anyway (which doesn't mean you can't add it here, too).

You can use the new API and we can add it (not backport). I would have guess that hte few times someone wanted it, they added it to their own file because they didn't want to wait for upstream to have it anyway (which doesn't mean you can't add it here, too).

Skimming through https://numpy.org/doc/stable/reference/c-api/iterator.html, it looks like this would be great as a separate PR so as to not block this one. I am happy to open one that adds the new iterator API to Cython.

I thought we had already put this in. For some reason force pushing a rebase to fix the merge conflict isn't working for me, though?!

@zoj613 could you maybe rebase? Sorry about that. Otherwise, this should just go in IMO @charris.

Could you try again? I think I had to toggle the "Allow edits and access to secrets by maintainers" option

Could you try again? I think I had to toggle the "Allow edits and access to secrets by maintainers" option

Thanks, worked.

This doesn't backport easily. There are failing tests, the names of have changed (_build) making comparisons difficult. I can probably fix things given time, but it would be easier to forego the backport.

EDIT: I'm tempted to tag a commit prior to the change to _build just for differencing.

Took this off the backport list.