Fyi @Campbell Barton (campbellbarton) and @Bastien Montagne (mont29) Bogdan works on the ProRender plugin with us at AMD. But this should be useful for most plugins/users of the Python api.

Besides points raised by @Jacques Lucke (JacquesLucke), generally LGTM, and nice addition indeed.

Wouldn't mind having @Campbell Barton (campbellbarton)'s check on this one too though, py code looks fine to me, but am far from being a Py C API master...

Thanks. The test should be added to tests/python/CMakeLists.txt as well. This way it will run when make test is run.

add_blender_test(
  script_pyapi_prop_array
  --python ${CMAKE_CURRENT_LIST_DIR}/bl_pyapi_prop_array.py
)

I also noticed that one test is failing:

Traceback (most recent call last):
  File "/home/jacques/blender-git/blender/tests/python/bl_pyapi_prop_array.py", line 27, in test_foreach_getset_i
    self.array_i.foreach_set(np.arange(10, dtype=np.int64))
AssertionError: TypeError not raised

It would be nice if you could provide some benchmark results.

I did comparison tests of reading image pixels to numpy array. I used 1024x1024 image. Getting pixels data took:

• using pixels.foreach_get() - 0.02 sec
• using np.fromiter(pixels, dtype=np.float32): 1.7 sec
• using np.array(pixels, dtype=np.float32): 1.8 sec

So, foreach_get is approximately 90 times faster.

I forgot to mention before that there is also a warning:

/home/jacques/blender-git/blender/source/blender/python/intern/bpy_rna.c: In function ‘pyprop_array_foreach_getset’:
/home/jacques/blender-git/blender/source/blender/python/intern/bpy_rna.c:5466:5: warning: enumeration value ‘PROP_BOOLEAN’ not handled in switch [-Wswitch]
 5466 |     switch (prop_type) {
      |     ^~~~~~
/home/jacques/blender-git/blender/source/blender/python/intern/bpy_rna.c:5466:5: warning: enumeration value ‘PROP_STRING’ not handled in switch [-Wswitch]
/home/jacques/blender-git/blender/source/blender/python/intern/bpy_rna.c:5466:5: warning: enumeration value ‘PROP_ENUM’ not handled in switch [-Wswitch]
/home/jacques/blender-git/blender/source/blender/python/intern/bpy_rna.c:5466:5: warning: enumeration value ‘PROP_POINTER’ not handled in switch [-Wswitch]
/home/jacques/blender-git/blender/source/blender/python/intern/bpy_rna.c:5466:5: warning: enumeration value ‘PROP_COLLECTION’ not handled in switch [-Wswitch]
/home/jacques/blender-git/blender/source/blender/python/intern/bpy_rna.c:5531:5: warning: enumeration value ‘PROP_BOOLEAN’ not handled in switch [-Wswitch]
 5531 |     switch (prop_type) {
      |     ^~~~~~
/home/jacques/blender-git/blender/source/blender/python/intern/bpy_rna.c:5531:5: warning: enumeration value ‘PROP_STRING’ not handled in switch [-Wswitch]
/home/jacques/blender-git/blender/source/blender/python/intern/bpy_rna.c:5531:5: warning: enumeration value ‘PROP_ENUM’ not handled in switch [-Wswitch]
/home/jacques/blender-git/blender/source/blender/python/intern/bpy_rna.c:5531:5: warning: enumeration value ‘PROP_POINTER’ not handled in switch [-Wswitch]
/home/jacques/blender-git/blender/source/blender/python/intern/bpy_rna.c:5531:5: warning: enumeration value ‘PROP_COLLECTION’ not handled in switch [-Wswitch]

Since you checked for those cases before, I think it is appropriate to simply add a default case with BLI_assert(false); break;.

Also, the test is still failing for me. In my case the reason is that an int64 buffer has the format letter l, which you seem to assume is int32 as well. I see that the Python documentation says that, but it's not true on my system. The tests runs successfully when I remove f != 'l' && from your code. If l is actually an int32 on your system it might be better to add an additional check like buf.itemsize == sizeof(int).

In D7053#169510, @Jacques Lucke (JacquesLucke) wrote:

I forgot to mention before that there is also a warning:

/home/jacques/blender-git/blender/source/blender/python/intern/bpy_rna.c: In function ‘pyprop_array_foreach_getset’:
/home/jacques/blender-git/blender/source/blender/python/intern/bpy_rna.c:5466:5: warning: enumeration value ‘PROP_BOOLEAN’ not handled in switch [-Wswitch]
 5466 |     switch (prop_type) {
      |     ^~~~~~

Since you checked for those cases before, I think it is appropriate to simply add a default case with BLI_assert(false); break;.

No, please DO NOT use default: here. This is straight way to Hell when adding a new type, and you forget to check all switch/cases. Please use an explicit stack of case PROP_ENUM: etc. instead.

No, please DO NOT use default: here. This is straight way to Hell when adding a new type, and you forget to check all switch/cases. Please use an explicit stack of case PROP_ENUM: etc. instead.

I tried to follow the code style of other functions bpy_rna.c file and there are lot of 'default' keywords there in switching property types. Also in my function there is a check in the beginning:

if (prop_type != PROP_INT && prop_type != PROP_FLOAT) {
    PyErr_Format(PyExc_TypeError, "foreach_get/set available only for int and float");
    return NULL;
  }

Of course I can do what ever you ask, but ARE YOU SURE that using explicit check of all types here is good idea?

Hello,
What is the resolution of this diff request? Is there anything I have to do here?
Thanks.

Besides the inline comment, this is looking good to me.

I do not fully agree that default must not be used here, because this code will not become wrong when a new property type is added. Not using default just means more work when a new type is added.

I'll commit this without default now, so that everyone is happy.

Thank you for this useful addition.
There's one gotcha that I think should be mentioned here for people like me struggling with foreach_get still being slow for Image.pixels
Since googling often suggests this page I'll post it here, hopefully it isn't inappropriate.

Major Gotcha: Do not ever copy pixels data from one image to another directly. This is 100x times slower. Use temporary buffer instead.

Slow example:

t0 = datetime.now()
# Save Viewer image data to temporary image
bpy.data.images['Viewer Node'].pixels.foreach_get(bpy.data.images.get("TempImage").pixels)
print("Time spent: %s seconds" % ( (datetime.now() - t0).total_seconds() ))

This single operation takes several minutes, depending on image size.

Fast example:

tempBuffer = numpy.empty(imageWidth * imageHeight * 4, dtype=numpy.float32)

t0 = datetime.now()
# Save Viewer image data to temporary image
bpy.data.images['Viewer Node'].pixels.foreach_get(tempBuffer)
bpy.data.images['TempImage'].pixels.foreach_set(tempBuffer)
print("Time spent: %s seconds" % ( (datetime.now() - t0).total_seconds() ))

This takes several seconds, for the same image size!