Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Defining method overload,JavaMultipleMethod, after autoclass #743

Closed
ismailsimsek opened this issue Jan 26, 2025 · 6 comments
Closed

Defining method overload,JavaMultipleMethod, after autoclass #743

ismailsimsek opened this issue Jan 26, 2025 · 6 comments

Comments

@ismailsimsek
Copy link

is it possible define overloaded methods, after autoclass?

MyClass = autoclass('io.MyClass')
# ensure both overloaded methods are recognised
MyClass.myMethod =  JavaMultipleMethod([
        '(Lio/Class1;)Lio/Builder',
        '(Lio/Class2;)Lio/Builder'
    ])

im passing and python implementation of an interface(io/Class2) but its not given to the right method.
it takes it as a io/Class1

@ismailsimsek
Copy link
Author

ok did it like below. got it working.

MyClass = autoclass('io.MyClass')
MyClass.myMethod =  JavaMethod('(Lio/Class2;)Lio/Builder'')

now calls to myMethod are using second of the overloaded method.

@cmacdonald
Copy link
Contributor

It might be good to have a test case if you think there is indeed a problem. there is some method selection code that tries to do the right thing...

Isnt this a case of just casting the object to the correct type? pyjnius.cast("Lio.Class1", obj)?

Or is this a problem with static methods?

@ismailsimsek
Copy link
Author

ismailsimsek commented Jan 28, 2025

@cmacdonald its a fairly complex usage. AFAIK not related to casting or static method

in this PR you could see full code, commented the fix.

@cmacdonald im worried about the correct usage of pyjnius . especially around the garbage collected classes and usage of proxy class. not sure if this code follows all the practices. If you have time, could you do a review of the code? will appreciate your review.

@cmacdonald
Copy link
Contributor

You have two notifying methods in DebeziumEngine$Builder.
If the code is calling the wrong one, you can use the signature= kwarg to specify the one you do want. See example at https://github.com/terrier-org/pyterrier/blob/master/pyterrier/java/_core.py#L147-L149

I'm confused about your code - you are overriding the notifying method with one from a different class. If its because the actual calling code is ending up calling DebeziumEngine$Builder.notifying() rather than DebeziumEngine$ChangeConsumer.notifying(), then you should try also casting to DebeziumEngine$ChangeConsumer first?

@ismailsimsek
Copy link
Author

ismailsimsek commented Jan 28, 2025

You have two notifying methods in DebeziumEngine$Builder. If the code is calling the wrong one, you can use the signature= kwarg to specify the one you do want. See example at https://github.com/terrier-org/pyterrier/blob/master/pyterrier/java/_core.py#L147-L149

exactly this is the case. i will check the example

I'm confused about your code - you are overriding the notifying method with one from a different class. If its because the actual calling code is ending up calling DebeziumEngine$Builder.notifying()

this is correct, however DebeziumEngine$Builder.notifying() has two notifying() methods with different signatures

  • first one: the one called without any fix.
    DebeziumEngine$Builder.notifying(Consumer<R> consumer)
  • second one: the desired one. proxy class is implementing ChangeConsumer therefor this method should be the one getting called. when the proxy class is given as an argument.
    DebeziumEngine$Builder.notifying(ChangeConsumer<R> handler)

Edit:
the call flow is like below:
io.debezium.engine.DebeziumEngine creates io.debezium.engine.DebeziumEngine$Builder
then it calls io.debezium.engine.DebeziumEngine$Builder.notifying(ChangeConsumer handler) to set the handler variable.

@cmacdonald
Copy link
Contributor

cmacdonald commented Jan 28, 2025

So I think you want something like:

builder = pt.autoclass("my.class")() # remember to construct the instance from the class
builder.notifying(handler, signature="(Lcom/mine/ChangeConsumer;)V")

More details:

JavaMultipleMethod calls calculate_score() to decide which method to invoke. Exact matches score highly; the assignability of an object to the class of an argument is not considered:

# if we pass a JavaClass, ensure the definition is matching
# XXX FIXME what if we use a subclass or something ?
if isinstance(arg, JavaClass):
jc = arg
if jc.__javaclass__ == r:
score += 10
else:
#try:
# check_assignable_from(jc, r)
#except:
# return -1
score += 5
continue

In short, the calculate_score() is a series of heuristics, trying to reimplement Java rules while inexactly matching to Python types at the same time.

Specify the signature simply tells jnius which method you want.

PS: Perhaps a PR for the documentation that tells people about signature kwarg?

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

No branches or pull requests

2 participants