Improve ScriptService to register discovered Plugins

[imagejan]

If scripts in a dedicated directory contain classes annotated with @Plugin, they should be automatically registered by adding them to the PluginService, so they can easily be called from other scripts.
See this gitter post and subsequent posts.

(This implies that the scripts would have to be parsed first, which might complicate things.)

[ctrueden]

This implies that the scripts would have to be parsed first, which might complicate things.

We do already parse all scripts upfront to extract the input and output parameters. Only if we find output parameter(s) which appear to represent plugin classes would we actually execute the script upfront to obtain said class(es) so that it can be registered.

[imagejan]

I'd love to make this happen, maybe together with the new parameter annotation from 9667fb9
Maybe we can discuss this on the upcoming DAIS learnathon in Dresden?

Something along these lines:

• Check for script outputs that are Plugin or can be converted to PluginInfo
• Auto-run the detected scripts
• Register the returned Plugins in the context

[ctrueden]

I'd be happy to discuss in Dresden, sure.

I would like to understand the use cases where you need this capability. The general goal is to allow defining Java classes in script languages and then register those classes as plugins, I understand. This is fine. However, there is a question of whether we can make it easier for the common cases. E.g., if what you want is to "write an Op in Jython", say, we might be able to make that happen without needing to explicitly define a Java class? Needs further thought and discussion.

[ctrueden]

/cc @dietzc

[imagejan]

After some discussion, here a more general use case: let's consider a script MyUtils.groovy containing some utility functions:

#!groovy
#@autorun=true
#@name="scijava.demo"

def helloWorld() {
  println "Hello World!"
}

String[] getNames() {
  return ["Curtis", "Christian", "Jan"]
}

If this script is parsed during startup, it should be run automatically, and the resulting functions should be usable from other scripts. (The question is whether it's required to be wrapped in a Class object, or can be run via its script name somehow.)

#@ScriptService scripts

/* Possible ways to import the static script methods ?? */
// import MyUtils
// import scijava.demo
scripts.import("scijava.demo")

println helloWorld()
println getNames()

@ctrueden @dietzc comments?

[ctrueden]

I think there are multiple different, but very related, use cases here: 1) autorun on start; 2) include as a library; and maybe (but maybe not) 3) autoinclude libraries.

Autorun

#@script(autorun=true)
#@name="scijava.demo"

def helloWorld() {
  println "Hello World!"
}

String[] getNames() {
  return ["Curtis", "Christian", "Jan"]
}

Include

#@script(name = "groovy-util")
def helloWorld() { println "Hello World!" }
String[] getNames() { return ["Curtis", "Christian", "Jan"] }

And then in another script:

#@include("groovy-util")
println helloWorld()
println getNames()

Autoinclude

#@script(name = "groovy-util", autoinclude = true)
def helloWorld() { println "Hello World!" }
String[] getNames() { return ["Curtis", "Christian", "Jan"] }

And then in another script:

println helloWorld()
println getNames()

But autoincludes have similar problems to autoimports—if the base library disappears or changes, the downstream code mysteriously stops working, with very little hint why.

[imagejan]

Should we improve the ParameterScriptProcessor#parseParam() method for this? Or add another ScriptProcessor plugin for that specific case?

One issue I see with your proposed syntax, @ctrueden:

#@script(name = "groovy-util")

What if one day, we want to have a shortcut #@ Script to define inputs of type some.library.Script?

So, should annotations like script and include have their own distinct syntax, or is it sufficient to have them lower-case only?

[ctrueden]

What if one day, we want to have a shortcut #@ Script to define inputs of type some.library.Script?

Yes, the conflation of syntax bothers me with this (also #@dependency, #@repository, etc.). But I am reluctant to introduce several different two-character prefixes, since A) it might be too easy to mentally confuse them, and B) it might increase the chance that a two-character sequence which is syntactically useful in some (present or future) script language becomes reserved by the SciJava preprocessor and hence unusable in that language.

I think it is pretty unlikely that we will someday have a class called Script that is a supported parameter type and that we want to alias to its short name. Same for Dependency and Repository. And even if so, as you say, we could differentiate on case.

I also think we should make a decision about whether we put a space after the #@. You seem to like it, and until now I have been writing no space. Let's be consistent everywhere. I'm fine with the space when it comes to script parameters. For #@repository and #@dependency, do you think those should also have a space?

We could also conceivably use that whitespace to distinguish between "an input of type script" and "a script metadata annotation." Because #@ script is really a shorthand for #@input script. So I lean toward no space when including the directive; it's just that for #@ script, the directive is an implicit input. I find this conceptually pleasing, but it's likely to be error-prone for new users.

[imagejan]

I like the idea of distinguishing directives (both #!groovy and #@dependency etc.) from inputs by the absence/presence of a space character.
I used space for script parameters so far because I think its more readable, and the ScriptProcessor allowed it 😄

We could be tolerant and keep accepting both #@int and #@ int for inputs, except for "reserved words" i.e. dependency, repository, input, output, script and include, where no space would be allowed. What do you think?

To avoid confusion, we should try to document as accurate as possible of course... and not introduce breaking changes in the future.

[ctrueden]

We could be tolerant and keep accepting both #@int and #@ int for inputs, except for "reserved words" i.e. dependency, repository, input, output, script and include, where no space would be allowed.

Great. I would phrase it more like: for reserved keywords, they take precedence over the shorthand. So to make the preprocessor recognize the word as a Java type, you can write either #@input script or #@ script since either of those does not match the #@script directive. And of course, we should not introduce any new directives matching existing aliased types.

To avoid confusion, we should try to document as accurate as possible of course... and not introduce breaking changes in the future.

OK, let's change any #@foo inputs in the wild to #@ foo (or #@input foo in the rare cases where we are highlighting the #@input directive) as we run across them. And I don't foresee needing to break this syntax in the future. (We can write that on my tombstone.)

[imagejan]

Great, so the next tasks:

• Create an IncludeScriptProcessor implementing ScriptProcessor (and possibly modeled after GrabScriptProcessor) that processes the script and include directives.
• Keep a list of script objects with name that can be re-used by other scripts (can we use the ObjectService for this?
• For included scripts, append the code to the current script before running, or simply execute the included scripts before executing the current script??

Sorry for asking this, I'm still unsure about how to implement this, but would like to try it eventually.

[ctrueden]

I'm still unsure about how to implement this, but would like to try it eventually.

We are talking about implementing #@include first, right?

Create an IncludeScriptProcessor implementing ScriptProcessor (and possibly modeled after GrabScriptProcessor) that processes the script and include directives.

Looking at GrabScriptProcessor for inspiration seems good, yeah. Tangentially: I see that currently, as written, the #@repository and #@dependency directives are allowed to have spaces. Should we change that now, before things get too far along? Based on our discussion above, I think we should enforce directives always having no intervening spaces.

Keep a list of script objects with name that can be re-used by other scripts (can we use the ObjectService for this?

The ObjectService keeps lists of objects differentiated on type. Differentiating on name is not a feature it has built in, but we can add some API to the ScriptService for that which is backed by a HashMap or similar.

For included scripts, append the code to the current script before running, or simply execute the included scripts before executing the current script??

After thinking a bit, I think we want to paste in the included code directly. Some arguments:

• This is what the C preprocessor does, I believe.
• It is also what MediaWiki template transclusion does.
• If you want to run a script, you can simply use scriptService.run(...) in your code. (We could add an API signature—or make the existing signatures smarter about this when the given path matches a known name—that lets you run a named script, since the ScriptService will be the thing keeping track of these named scripts anyway.)

Alternately, I guess the #@include directive could have an option like:

#@include(name="groovy-util", transclude=false)

Although before doing that, it would be good to articulate the concrete pros and cons of transclusion vs. execution.

[imagejan]

Based on our discussion above, I think we should enforce directives always having no intervening spaces.

Yes, I'd be in favor of this, as it makes a clear distinction between directives and inputs (where the input directive is implicit and can be left out).

We are talking about implementing #@include first, right?

Yes. I wonder what use cases we should support for using a common script library.
The simplest from a user perspective would be to allow the definition of method on a script level, and after inclusion make them available on script level again (this is transclusion, right?):

Library script:

#@script(name="mylib")
def myAddFive(number) {
	return (number + 5)
}

Usage script:

#@include(name="mylib")

result = myAddFive(3)

Execution

Execution wouldn't currently work with the above use case, as the defined function isn't available in the scope of the second script. Likewise, this example currently doesn't work:

#@ ScriptService scripts

script = """
def myLibraryFunction(text) {
	println text
	return 42
}
"""

scripts.run("foo.groovy", script, true)
result = myLibraryFunction("Hey")
println result

---

Transclusion

The issue I see with transclusion is that in order to support all languages, we would have to copy the included code at the beginning of the script, which leads to inconsistent line numbering when exceptions are thrown.
Defining functions at the end of a script (as it is done when using the ImageJ 1.x Interpreter.setAdditionalFunctions()) would work in IJ1 Macro and Groovy, but not in Python:

print myLibraryFunction("Hey")

def myLibraryFunction(text):
	print text
	return 42

---

Alternatively, we could aim for a bit more complex (but concise) syntax like:

#@include(name="mylib")

result = mylib.myAddFive(3)

In this case, we can create a mylib class with the defined methods. Is this what you intended?

[imagejan]

@ctrueden wrote:

I see that currently, as written, the #@repository and #@dependency directives are allowed to have spaces.

Well, they will be parsed when containing a space, but the following lines only match when line.substring(2, paren) is the directive, without any spaces around them, no?

https://github.com/scijava/scijava-grab/blob/7ee3cbdf32b5d24ede14634ff633b369eba78012/src/main/java/org/scijava/grab/script/process/GrabScriptProcessor.java#L83-L95

Also, the current pattern does not allow spaces between the directive and the following parenthesis, which is an unneccesary restriction IMHO.

When I find the time, I'll submit a PR changing the pattern and trimming the directive String before the switch statement.

---

Another question: I can't see a difference in priority for GrabScriptProcessor and ParameterScriptProcessor, so the former seems to work only because the latter doesn't find any matching type, right? Should we give ScriptProcessors for directives a higher priority than ParameterScriptProcessor?

---

An argument for execution instead of transclusion: you would be able to include scripts in a language-agnostic way, i.e. write a "library" with static utility methods in Python and re-use it like a Java class with static methods from Groovy, etc.

[ctrueden]

After discussion with @imagejan, we have filed new issues relating to the above discussion and tangential topics:

• Implement #@script directive with most @Plugin attributes #294 - Implement #@script directive with most @Plugin attributes
• Implement #@import directive #295 - Implement #@import directive
• Add #@--- directive #296 - Add #@--- directive
• Ensure all #@ directives do not allow a space before the directive #297 - Ensure all #@ directives do not allow a space before the directive
• Add autorun={"true", "false"} attribute to #@script directive #298 - Add autorun={"true", "false"} attribute to #@script directive

These issues cover everything originally desired by this issue; hence, this issue is now closed in favor of the new ones.

[imagejan]

Additionally, issue #200 was updated to reflect the latest state of the discussion.