"Generic" JArray support #1214
"Generic" JArray support #1214
Conversation
astrelsky
force-pushed
the
jarray
branch
2 times, most recently
from
2656cb4
to
5f37d4f
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1214 +/- ##
==========================================
- Coverage 87.21% 87.21% -0.01%
==========================================
Files 113 113
Lines 10281 10293 +12
Branches 4065 4066 +1
==========================================
+ Hits 8967 8977 +10
- Misses 718 719 +1
- Partials 596 597 +1 ☔ View full report in Codecov by Sentry. |
| @@ -339,6 +339,7 @@ def initializeResources(): | |||
| _jpype._type_classes[object] = _jpype._java_lang_Object | |||
| _jpype._type_classes[_jpype.JString] = _jpype._java_lang_String | |||
| _jpype._type_classes[_jpype.JObject] = _jpype._java_lang_Object | |||
| _jpype._type_classes[_jpype.JClass] = _jpype._java_lang_Class | |||
There was a problem hiding this comment.
This seems reasonable. Though technically all uses of _type_classes needs checking. There used to be an edge case though I don't see it any more. The only user is _jarray which i checked.
|
For some reason github ate part of my review and I had to repost. I always forget to hit start review before commenting. Sorry if there are repeats. |
|
I would recommend trying
jpype.JArray(_jpype._JClass)
to hit that code path. The backend classes all have unoccupied java slots unless the front end has specifically added it during initialization.
|
You are correct. I just checked and it's actually already protected here. jpype/native/python/pyjp_module.cpp Line 408 in 66f8c6c However, if I do Would you prefer I explicitly check for this case and raise the original TypeError? I can see how the error message in that specific case could confuse someone. |
Unfortunately isinstance will fail on some things such as typing.Collection. Using typing.Generic will prevent confusion.
|
If there is an option to squash the commits, may as well use it. I don't see it from my end. Don't worry about the "Unverified" code signature I have something messed up on my end and can't be bothered to deal with it. |
|
@Thrameos - this PR is good to go from my perspective. I appreciate you are deep in Python 3.13 work, so let me know if there is anything I can do to help with this. |
|
@pelson I think long term we need to either formalize the Unstable with extra calls such that they work and get them to move the extra data that they are adding into the preheader space. Unfortunately, I tested heavily with the allocate with extra memory version they provided and it didn't work In addition the with extra memory is fundamentally incompatible with most recent changes in Python 3.13. I am also happy if they just adopted the user data patch which accomplishes the same thing, Eventually they will get a stable object model which provides for true multiple inheritance and this sort of problem won't haunt us every version. I am not sure what it will take to get there. I can provide some patches or instructions for patching Python which we can provide for Python 3.14. Moving the inline dict to its proper space was only about 10 lines. (changing 2 macros, adding a type slot for the preheader allocation, and then the setting the free point.) But if we can't find a way to get patches accepted it won't get better. I am unfortunately too high a stress of person to navigate the bureaucracy that is Python (and maintain my very high stress job.) |
|
Thank you @astrelsky for your first contribution and taking the review process. |
This allows the use of a parameterized
JArrayin type annotations. There was no other way to represent a java array type.This also includes two small fixes where you could not do
JArray(JClass)orJClass[0].