Skip to content
This repository has been archived by the owner on Feb 4, 2022. It is now read-only.

Separate imagej from scijava components #67

Closed
hadim opened this issue Jun 2, 2017 · 5 comments
Closed

Separate imagej from scijava components #67

hadim opened this issue Jun 2, 2017 · 5 comments

Comments

@hadim
Copy link
Contributor

hadim commented Jun 2, 2017

  • remove unnecessary dependencies.

Need to have a working scijava-grab.

@hadim
Copy link
Contributor Author

hadim commented Jun 3, 2017

What needs to be done on the scijava-grab side? It seems to work pretty well to me.

@ctrueden
Copy link
Member

ctrueden commented Jun 3, 2017

The basic grabbing works very well. However, as you know, SJJK ships SciJava-based artifacts, including ImageJ, at a specific version. What if you grab net.imagej:imagej at a different version? That one should take precedence, IMHO, so that notebooks are truly reproducible regardless of the version of SJJK (barring bugs and new features, of course).

Furthermore, there is already an existing Context instance managed by SJJK, which needs to be modified any time an artifact is grabbed which has new SciJava plugins in it. Otherwise, you could grab net.imagej:imagej but then ImageJ services would still not be available to the context. But right now, modifying existing Context instances is pretty limited in what you can do. It is possible to add new plugins, but if services are already initialized, they may not behave as they would have had those plugins been on the classpath before initialization.

Rather than worrying about Context mutability too much though, a compromise we could implement without too much trouble is to defer creating the Context until the first usage of #@ parameters. This is a little bit thorny bootstrapping-wise, because it is the contextual ScriptService which knows how to parse the parameters. We could either:

  1. Use a rudimentary heuristic to look for parameters without a context existing; or
  2. Make a Context right away, but then any time a #@grab happens, make a new Context and throw away the old one.

I am leaning toward (2) right now, even though it might lead to mysterious behavior in certain cases. But we can encourage people to do their grabs up front in the initial cell(s), and then do all the actual cool stuff subsequently once the context has "settled down."

The other thing I'd like to address in scijava-grab soon is scijava/scijava-grab#1, to use the Maven local repository cache instead of a separate Groovy grape repository.

@hadim
Copy link
Contributor Author

hadim commented Jun 3, 2017

I understand better now. If (2) can work I am good with that. But is that even possible to replace a Context ? What gonna happen to all the SciJava objects that manage scripting for the kernel for example?

@ctrueden
Copy link
Member

ctrueden commented Jun 3, 2017

What gonna happen to all the SciJava objects that manage scripting for the kernel for example?

My initial idea was to simply throw them away and rebuild them. So any cell state would go away after a #@grab. That's why it is best to do the grabs up front. We would need to take care, though, to preserve the ClassLoader after the grab injects its new stuff; I haven't looked carefully enough at how scijava-grab works to know whether that would "just work", or be easy, or be tricky.

Tangentially: I think we can probably cut down on the amount of work that SJJK is doing to manage ScriptLanguages and ScriptEngines, with some very small changes to SJC. Now that the #! syntax can decide the language on the SJC side, SJJK doesn't really need to know about individual ScriptLanguage plugins at all anymore.

@ctrueden
Copy link
Member

We decided to discontinue SJJK in favor of using BeakerX's kernels directly. Using the Groovy kernel, all SciJava and ImageJ stuff can be grabbed, so no more need to stress over the bootstrapping issue described here.

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

No branches or pull requests

2 participants