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

Minor python issues resolved #94

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

sgimmini
Copy link

@sgimmini sgimmini commented Sep 7, 2022

There are some minor issues with the example python codes:

  1. a deprecation warning for using numpy.int() instead of the suggested native python int
  2. a TypeError using "old" java Strings. JVM needs an argument to convert these, to work in python

@jlizier
Copy link
Owner

jlizier commented Sep 8, 2022

Thanks for this.

  1. I found this one the other day and it's already in a commit that I haven't pushed yet. So am going to delete it from here to save unrolling the other parts I've got coming.
  2. This one I've not seen. Thanks for the links on those, I understand why it's happening. Am not completely sure why the jpype documentation says that it's "NOT" recommended for newly developed code, but I think this is about retaining maximum flexibility in differentiating Java and Python strings. We'd prefer the straight conversion though to make it easier for users (max flexibility not required), so will go with this solution. I'm also going to bring this into the AutoAnalyser in a commit being pushed shortly.

as these are already committed in my branch and about to be pushed ...
jlizier added a commit that referenced this pull request Sep 8, 2022
@jlizier
Copy link
Owner

jlizier commented Sep 8, 2022

Hmmm - it seems that me pushing my changes first was the wrong option, because it looks like it needs a rebase at your end now? Simon can you take a look and see if a merge and push at your end allows be to accept this again?

@Thrameos
Copy link

Thrameos commented Sep 8, 2022

The reason it is not recommended is that it forces all strings to Python even if they are just passing through which is slow and loses the expected Java methods. If you have a larg list of strings string and you are going to just check if it each ends in dot, then that is a big waste of lime converting. For reuseable modules that are meant to work with others convertStrings should be false. For a standalone program it is up to programmer to decide.

The issue isnt so much JPype as Python as strings are forced to be concrete. If they were abstract then they could be both a Python and Java string at the same time and only have lazy conversion on the first use. The original JPype 0.6 was converting all which was found to be a problem, so the recommended and default behavior were switched.

@sgimmini
Copy link
Author

The file was removed from the working tree, but I simply restored the one I pushed. Should work now I hope!

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.

3 participants