Many methods accept both numpy arrays and Python lists are parameters, but the method signatures only mention numpy arrays. Can this be improved? #584
Comments
hx2A
added
enhancement
|
I think it's questionable whether you even need set_fills(). So far I can't come up with a demo which shows how to turn it loose on a shape and expect it to find all the faces. It looks at the number of vertices and not the number of faces. For example, a cube has 6 faces, but only 4 vertices for each face and it generates this error if you call it on a PShape cube: |
I agree, and I didn't want to add
You'd need to get vertex count with |
|
Would that set_fills() accept an argument of type array.array as well? Maybe even Java arrays returned from Java functions; or created by java.lang.reflect.Array.newInstance() static method? |
|
I think An alternative would be to have some vectorized drawing methods that can take shapes + attributes (this is something we discussed when talking about rendering geodataframes...). It would be cool if |
Yes, absolutely! It can work with Java arrays. Adding that to the signatures would make them even more complicated though. My preference is to work with numpy arrays. JPype does let you create numpy arrays that are backed by a Java array. It uses memory buffers. The data will be read only but there is no memory copying, so the performance is very fast. |
Well, one of the difficulties is because of the bug. It works well for what it was designed to do, which is efficiently set the fill colors during shape creation so py5 can quickly convert Trimesh objects to something py5 can use.
I would suggest using hybrid coding for this. It's like a way of making fast and efficient Java extensions for py5. They can be customized for your use case. |
It occurred to me this would be a way of doing it, probably "the" way, but I was too lazy to engage with these ideas. Maybe in 2025 :D |
2025 will be the year of big things! |
Why not instead use a broader datatype hint like I haven't checked set_fills()'s implementation; but my guess would be it simply iterates over its fills parameter, right? If it requires direct access to fills's indices, a more specific, but still broad, type hint I've done some tests on how "jpype" perceives Java's arrays & arraylists: from array import array
from typing import Iterable, Sequence
import jpype # from 'jpype1' package
# Function to check if an object is a Java array
def is_java_array(obj):
return obj.getClass().isArray() if isinstance(obj, jpype.JObject) else False
# Start the JVM
jpype.startJVM()
# Import the Array classes
Array = jpype.JClass('java.lang.reflect.Array')
Arrays = jpype.JClass('java.util.Arrays')
Vector = jpype.JClass('java.util.Vector')
# Create a Python tuple for testing
python_tuple = 10, 20, 30, 40, 50
# Create a Java ArrayList from the Python tuple
java_list = Arrays.asList(python_tuple)
# And convert it to a Java Vector, which is also a List-like container
java_list = Vector(java_list)
# Create a Java array and populate it w/ the contents of the Java Vector
java_array = Array.newInstance(jpype.JInt, len(java_list))
for i in range(len(java_array)): java_array[i] = java_list[i].intValue()
# Create a Python array for testing using the Java array
python_array = array('i', java_array)
# Test the function is_java_array()
print(java_array, len(java_array), is_java_array(java_array), type(java_array)) # True
print(java_list, len(java_list), is_java_array(java_list), type(java_list)) # False
print(python_tuple, len(python_tuple), is_java_array(python_tuple), type(python_tuple)) # False
print(python_array, len(python_array), is_java_array(python_array), type(python_array)) # False
# Check if Python believes a Java array is an Iterable or a Sequence
print("\nJava's array is an Iterable:", isinstance(java_array, Iterable)) # True
print("Java's array is a Sequence:", isinstance(java_array, Sequence)) # False
# Check if Python believes a @RandomAccess or a List-like Java container is an Iterable or a Sequence
print("\nJava's ArrayList is an Iterable:", isinstance(java_list, Iterable)) # True
print("Java's ArrayList is a Sequence:", isinstance(java_list, Sequence), '\n') # False
# Check if a Java array behaves as a Python Iterable
for int in java_array: print(int)
# Shutdown the JVM (optional)
jpype.shutdownJVM()It outputs the following: Seems like "jpype" recognizes a Java array as an Iterable, but not as a Sequence though. So I've opened an issue about it on "jpype"'s repo: |
Here is the link to that code. It just uses a for loop to iterate over the passed array. It is much more efficient to iterate through a list like this in Java. The alternative is to loop in Python and make many individual calls to Java to change one vertex color at a time. That is slow.
This is a good idea! Thank you for the suggestion. I am looking into making this change now. |
|
There are many methods like https://py5coding.org/reference/sketch_points.html The type hint there could perhaps be A list of tuples does actually work for |
Yep, I've used lists of tuples quite a few times with |
Interesting, so the whole set_fills(), and other similar methods, are split in Python & Java code! After taking a careful look at both implementation halves, I believe B/c parameter fills has to be forwarded to the Java half as a IMO, it's unlikely users would pass an actual generator/iterable as argument to set_fills(). Most users would just pass a So such lines like Rather, for the rare cases a user would wanna pass a generator to set_fills(), the user should use the unpack/spread operator Or explicitly call list() or tuple() over the argument. BtW, I did some quick refactoring for both halves. What do you think? def set_fills(self, fills: Sequence[int], /) -> None:
"""$class_Py5Shape_set_fills"""
if fills: _Py5ShapeHelper.setFills(self._instance, fills) public static void setFills(final PShape s, final int[] fills) {
if (fills.length != s.getVertexCount()) throw new RuntimeException(
"the length of parameter fills (" + fills.length +
") must be equal the number of vertices in the shape (" + s.getVertexCount() + ")"
);
for (int i = 0; i < fills.length; s.setFill(i, fills[i++]));
} |
I quite like the idea of being able to use a generator expressions, and this is exactly what I did here but I would accept the limitation if it would improve things otherwise. |
I agree. I added that code that converts a generator to a list because JPype won't automatically do that to a generator in its attempt to convert the parameter to a Java array. Instead it will give a confusing error.
Yes! And much of py5 is like this. Nobody interacts with the PApplet class directly; there are Java Sketch and SketchBase classes that help facilitate a comfortable interaction between Python and Java. On the Python side there is auto-generated code for much of it with extra decorators added to make small adjustments. The whole thing works very well and is easy to maintain. for (int i = 0; i < fills.length; s.setFill(i, fills[i++]));Ah, I like that! I never thought to add code into a for loop like that. |
Oh, I see! You have code lines like these there: Although those could easily be changed to a "list" comprehension syntax by using Which would relief those methods the need to have B/c regardless, your generator comprehension argument would still need to be checked and then converted to a list or tuple before invoking the internal Java method. And for everything else, there's still the permanent cost of always checking if it happens to be a generator instead of a sequence container. |
|
On the other hand, @GoToLoop, Python's |
Yes, but for most cases the time to do the memory copy to send the data from Python to Java will be larger, so it's OK. |
|
This is now fixed. Updated reference docs with all of the changes have been pushed to the dev website. EG: http://dev.py5coding.org/reference/py5shape_set_fills.html Here's the full list of methods with new signatures: Py5Graphics.bezier_vertices() There also were a couple of signature bugs I picked up on along the way. Revisiting this part of py5 helped improve the docs quite a bit. Thanks to everyone who contributed to the discussion here and elsewhere. |
|
AFAIK, neither lists, tuples, arrays, etc. are Iterator types, but Iterable & Sequence! from typing import Iterator, Iterable, Sequence
empty_list, empty_tuple = [], tuple()
# False True True
print(isinstance(empty_list, Iterator), isinstance(empty_list, Iterable), isinstance(empty_list, Sequence))
# False True True
print(isinstance(empty_tuple, Iterator), isinstance(empty_tuple, Iterable), isinstance(empty_tuple, Sequence))But IMO, Sequence would be the best datatype hint for them. |
Ooops, it looks like I confused
I just looked up the difference between |
|
If you always converted things with |
Right, but to add that to everywhere it would need to be added would be tedious and difficult, and right now I'm not sure I should have added that conversion to the places where it has been added already. I don't think it is a good idea to modify the user's passed parameters. If I have a generator and pass it to one of these methods, the generator will be "spent" as you cannot rewind a generator. That's probably why JPype doesn't do this when you pass a generator. It's a weird side effect. I think it would be better to make the user add brackets ie |
|
Hmmm... I'm not sure I understand your reasoning... this part of the Python vocabulary is rather confusing anyway. But yes the generator will always be consumed/spent and this is expected either way, converting it to a tuple or inside a loop etc. but never mind! Any solution you adopt, if it is well documented will be fine! |
|
Well I guess if someone writes code with the generator in the method call, like this: shape.set_fills(py5.color(py5.random(255), 0, 0) for _ in range(shape.get_vertex_count()))people would expect the generator would be created and consumed right there. But if the generator comes from somewhere else: gen = (py5.random(255), 0, 0) for _ in range(shape.get_vertex_count()))
...
shape.set_fills(gen)
# gen is now emptythen it could be unexpected. But on the other hand, assigning generator objects to variables like that is a more complex thing to do, and someone doing that would be well aware of what It does bother me a bit that there is an inconsistency. For example, right now you can pass a generator to |
|
OK, now this is fixed. Everything that was http://dev.py5coding.org/reference/py5shape_set_fills.html
Turns out I already thought of this and took care of it a long time ago. So although Python lists do work for these methods, numpy arrays are still the preferred way to go here because the performance will be much better. |
What if we pass an actual Java |
Yes, that would also work fine, and would be fast and efficient because there is no memory copying from numpy to Java. Although you pointed out that Java arrays are not Personally I find working with Java arrays in Python to be irritating. In particular, in Java, pixels are the one dimensional array of ints. Pixels in numpy with |
It means for the time being, if we prefer to pass a Java array, we'll have to silence the linter for the calling line, until JPype devs add BtW, now that you have type |
Oh right, linters will pick up that.
Yes, absolutely. I would not revoke functionality like that. I can't easily change the type hints on those to |
So the actual type hint for set_fills()'s fills parameter is still: And IDE linters will complain about this @villares's code line In other words, the type |
Yes, that's a good way of putting it. |
As mentioned in a discussion thread:
Originally posted by @vsquared in #577 (reply in thread)
There are methods that accept both numpy arrays and Python lists are parameters, but the method signatures only mention numpy arrays. Can this be improved without making the signatures nasty type Unions? What is most clear for users?
The text was updated successfully, but these errors were encountered: