Skip to content

Conversation

cmacdonald
Copy link
Contributor

In #753 (comment) it was suggested by @da-woods that the fix for #753 was sub-optimal:

ctypes.c_long probably isn't the right fix. isinstance(x, long) was testing for the Python 2 long type (which no longer exists in Python 3 - Python 3 just has one integer type that covers both large and small integers).

This PR:

  • removes ctypes.c_long
  • when scoring method signatures, it better tests argument value ranges for compatibility for both short and integer types
  • adds some more test cases
  • adds a debug kwarg for method and constructor invocation that displays the scores of the different methods that are being scored in calculate_scores(). This may be useful when use of the signature kwarg is necessary.

Example output:

>>> clz = autoclass("org.jnius.SignatureTest$IntOrLong")
>>> obj = clz(0, debug=True) # could be int or long
[(20, '(I)V', 'V', ('I',), (0,)), (20, '(J)V', 'V', ('J',), (0,))]
Selected (J)V for invocation

(ie. constructors (I)V and (J)V both got score of 20, jnius selected (J)V.

@cmacdonald
Copy link
Contributor Author

All failures are Pypy 3.8, which Cython 3.1 may not support?

@da-woods
Copy link

All failures are Pypy 3.8, which Cython 3.1 may not support?

I don't remember if it does or not off the top of my head, but I don't think anything in this change should need Cython 3.1. On earlier versions of Cython, long didn't do a lot except on Python 2.

@cmacdonald
Copy link
Contributor Author

All failures are Pypy 3.8, which Cython 3.1 may not support?

We already looked at this in #752: see #753 (comment)

@T-Dynamos
Copy link

T-Dynamos commented Jul 26, 2025

CC: @misl6

@misl6
Copy link
Member

misl6 commented Jul 27, 2025

Hi @cmacdonald, I've just pushed some changes that should fix the CI/CD pipeline, can you please rebase on top of master?

@cmacdonald
Copy link
Contributor Author

rebased!

Copy link
Member

@misl6 misl6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you!

@misl6 misl6 merged commit 8407fc7 into kivy:master Jul 27, 2025
134 checks passed
@misl6
Copy link
Member

misl6 commented Jul 27, 2025

@T-Dynamos + @cmacdonald , I guess we're only missing Python 3.13 support (and maybe Cython < 3) removal to process a release?

@T-Dynamos
Copy link

@misl6 Yes, I believe this is ready for release. I've also tested this PR on Android.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants