Skip to content

Prevent error when JVM is already running.#8

Closed
hadim wants to merge 1 commit intoscijava:masterfrom
hadim:warn
Closed

Prevent error when JVM is already running.#8
hadim wants to merge 1 commit intoscijava:masterfrom
hadim:warn

Conversation

@hadim
Copy link
Copy Markdown
Contributor

@hadim hadim commented Mar 19, 2019

See imagej/pyimagej#29 for details.

@hanslovsky
Copy link
Copy Markdown
Member

I do not agree that this check should be the responsibility of scyjava. As a (fairly) low-level library that does not do any UI work itself, I think that scyjava should fail if used incorrectly. The exception holds valuable information about improper use, which might get lost if only a warning is emitted and the python logging system has not been initialized.

What is the reasoning for not checking for the ValueError exception down-stream? If that exception is too broad, we could create a new exception and re-throw.

@hadim
Copy link
Copy Markdown
Contributor Author

hadim commented Mar 19, 2019

I understand your point.

But running twice the same line without modifying anything else and get an error only the second time is definitively not the correct pattern in my opinion.

I know the scyjava/pyimage/jnius init mechanism is complex but we should be able to run (or the equivalent scyjava code):

import imagej
imagej.init()

twice without running into an error.

Do you guys (@ctrueden and @hanslovsky) think this issue could be addressed elsewhere? Or do you think that issue should only be handled at the user level?

@hanslovsky
Copy link
Copy Markdown
Member

But running twice the same line without modifying anything else and get an error only the second time is definitively not the correct pattern in my opinion.

I agree but that is a general problem with (stateful) singletons: Running the same thing multiple times will produce different result depending on the global state.

Do you guys hink this issue could be addressed elsewhere?

imagej.init() looks like a good candidate to me. The scyjava_config portion of the code could be run only if pyjnius is not initialized.

@hadim
Copy link
Copy Markdown
Contributor Author

hadim commented Mar 20, 2019

For the record: imagej/pyimagej#30

@hanslovsky
Copy link
Copy Markdown
Member

Thanks @hadim

jschneidereit pushed a commit to jschneidereit/scyjava that referenced this pull request Jul 21, 2024
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