Skip to content

Use Jrun to manage (maven) Java dependencies#4

Merged
hanslovsky merged 5 commits intomasterfrom
jrun
Nov 13, 2018
Merged

Use Jrun to manage (maven) Java dependencies#4
hanslovsky merged 5 commits intomasterfrom
jrun

Conversation

@hanslovsky
Copy link
Copy Markdown
Member

Example:

import scyjava_config
import time

scyjava_config.add_endpoints('net.imagej:imagej')
scyjava_config.set_verbose(2)

from scyjava import jnius

ImageJ = jnius.autoclass('net.imagej.ImageJ')
ij = ImageJ()
ij.ui().showUI()

while True:
    time.sleep(0.5)

Fixes #3

@ctrueden
Copy link
Copy Markdown
Member

Super cool!

A couple of questions:

  1. I see that a substantial part of scyjava_config is a wrapper around jnius_config. Is it a goal to encapsulate the jnius dependency? If so, should we also wrap autoclass? As it stands, the jnius dependency still leaks.

  2. Is this naming foo_config for module foo a standard thing with Python? It seems weird to me as a newcomer; I haven't seen it in core Python modules. What about nested modules like scyjava.config? Which one is better style-wise?

  3. How do you see the type conversion logic I wrote fitting in? Do you have a preferred way of doing naming that would be Pythonic? Let me know and I'll start migrating the code.

@hanslovsky
Copy link
Copy Markdown
Member Author

  1. The goal is not to encapsulate the jnius dependency. For config purposes, I wanted to avoid having to import both scyjava_config and jnius_config so users can call scyjava_config.add_endpoint and scyjava_config.add_classpath instead of jnius_config.add_classpath.
  2. It is not standard but necessary to avoid import scyjava before import scyjava_config, see for example this post on stackoverflow.
  3. Not entirely sure as both imagej and imglyb are supposed to be downstream dependencies. Shouldn't they live in their own repositories?

@hanslovsky
Copy link
Copy Markdown
Member Author

As for pythonic styles: I am not python expert, but I think if we follow PEP (lower case method names, underscores for readibility; camel case class names), we should be fine. I would probably not add j in front of Java class names and just use their unqualified name, e.g.

RandomAccessibleInterval = jnius.autoclass('net.imglib2.RandomAccessibleInterval')

@ctrueden
Copy link
Copy Markdown
Member

@hanslovsky Thanks for the clarifications.

Shouldn't they live in their own repositories?

There are two layers of conversion logic: 1) stuff for converting between core Python and Java types, particularly collections; and 2) stuff for converting image data types. I kept them separated; the imagej/convert.py class on the smart-convert branch of imagej/imagej.py currently has the base conversion layer that I think belongs in scyjava.

Would you be willing to take a quick look and let me know where in scyjava you think makes sense for the functions currently in convert.py? Which .py file, with which function names? I really have no clue about Python naming, since I do not understand how Python modules are normally structured—when I look at https://github.com/imglib/imglyb/tree/master/imglyb I get confused, since I don't understand how those files line up with the import statements in calling code. I'm also happy to RTFM if there is one you recommend.

I would probably not add j in front of Java class names and just use their unqualified name

I agree. The reason I put the j is because collections has some classes with the same names as the Java ones, so there is a name clash. To avoid that, we can stick with import collections and then just reference the classes fully qualified like collections.Iterable etc.

Regarding this PR: all looks awesome; please merge whenever you want, or ping me if you would rather me do it.

@hanslovsky
Copy link
Copy Markdown
Member Author

hanslovsky commented Nov 13, 2018

I am not a python expert either and usually I just follow my gut feeling for project structure. I also do not think that this matters too much for a small project like this. That being said, this is how I would proceed:

  • Add convert.py with a few modifications:
    • convert seems like a good name to me, no objections here
    • Indicate methods that are meant to be private with leading underscore. There is no private in python but convention is that leading underscore means something like internal use. I assume everything but to_java and to_python can be considered private, no?
    • JavaNumber should be a method java_number rather than a class.
    • name all class object variables with their unqualified java name, prepend underscore if private
  • Add line to __init__.py (has to come after the call to _init_jvm()): from .convert import to_java, to_python.

Further reading:
https://docs.python-guide.org/writing/structure/
https://python-packaging.readthedocs.io/en/latest/minimal.html

@hanslovsky hanslovsky merged commit 8dc66ac into master Nov 13, 2018
@ctrueden ctrueden deleted the jrun branch November 13, 2018 18:03
@ctrueden
Copy link
Copy Markdown
Member

Thanks for the suggestions, @hanslovsky. I think I got it all complete and committed with 3bc7746 and c112aef!

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.

2 participants