getInputs() error on functions with no parameters

[nick-ulle]

Calling getInputs() on a function with no parameters produces an error:

getInputs(function() {})
# Error in validObject(.Object) :
#   invalid class "ScriptNodeInfo" object: invalid object for slot "outputs" in class "ScriptNodeInfo":
#   got class "NULL", should be or extend class "character"

The error goes away if you add a parameter to the function.

[duncantl]

Thanks. Fixed to avoid the error. Still deserves some thought about what the result should actually be.

[gmbecker]

@duncantl, was going to look at this today, but thanks for taking care of it. Currently the way things work, AFAIK, is that there is a ScriptNodeInfo for the function parameters (apologies for the no parameters case oversight if that was me), then there seems to be a node just for "{", at least in the version I have installed locally right this second, which is useless and we may not want it don't want, then there are one or more (really zero or more, I guess? though that seems like a pretty useless corner case) nodes for the body. The parameters scriptnodeinfo is there so that all the parameters don't look like globals. Currently the parameters node isn't specially classed, but it easily could be, and that might be useful (particularly for compiling?) if we think a bit about how to add modelling default values in there. One question is whether we want to skip the parameters nodeinfo if the function takes no arguments, or if we want it in there but just containing no outputs. I would think that if we were going to model the parameters with a specialized subclass, we'd want it to always be there (honestly, we might want a FunctionInfo subclass of ScriptInfo, as well if we're going that route), and just be empty in the no parameters case. Given that we're currently not, I dont' know that it makes too much difference, but I can;t think of any problems that having it always there would cause, so I would lean that way for consistency/future-proofing. This is just spitballing off the top of my head, but if we did go the specialized nodeinfo class route, you could imagine using that as a meaningful marker of scope somehow if things ever got that fun and complex, probably in the recursively analzying functions that are called in a script case. I'd have to think more about how useful that might be though. Anyway, to get to the concrete question, then, my vote is for getInputs(function(){}) to return either only one scriptnodeinfo with no inputs or outputs representing the parameters block, or the empty parameters block plus a node for `{`. Other thoughts?

…

On Thu, Oct 18, 2018 at 8:28 AM, Duncan Temple Lang < ***@***.***> wrote: Thanks. Fixed to avoid the error. Still deserves some thought about what the result should actually be. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#39 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA3dsVRE5UJWDKRO3aN_3umQBPhnbpFYks5umJ4KgaJpZM4XsNWL> .

-- Gabriel Becker, Ph.D Scientist Bioinformatics and Computational Biology Genentech Research