createMetaModule generator

[tech4GT]

Builds up on #765 , fixes #200, fixes #315

[tech4GT]

@jywarren So dow we want to support expanding meta modules? I say let's just stop supporting that API entirely and move all meta modules to this new architecture, what do you think?

[tech4GT]

Also I am returning the complete module from createMetaModule so if the function is called from node context by a user like image-sequencer-app we can the load it directly by passing it to the loadNewModule

[jywarren]

Oh, yes, that sounds right on both counts. Thanks Varun!!

…

On Sat, Feb 16, 2019, 4:41 AM Varun Gupta ***@***.*** wrote: Also I am returning the complete module from createMetaModule so if the function is called from node context by a user like image-sequencer-app we can the load it directly by passing it to the loadNewModule — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#779 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJy4mG5Glejf7gnj0rOv-E5fLrbRZks5vN9IigaJpZM4a-5T_> .

[tech4GT]

@jywarren Should I open a new pr removing the boilerplate code from modules or should we do it here itself?

[jywarren]

Well, it demonstrates how to set defaults and how to map inputs to the internal modules, so I think it's a good example. We might also show what happens if you don't supply a map function. For example what if there are two internal brightness steps? What appears externally as the parameters you can set in the UI?

…

On Sat, Feb 16, 2019, 4:48 AM Varun Gupta ***@***.*** wrote: @jywarren <https://github.com/jywarren> Should I open a new pr removing the boilerplate code from modules or should we do it here itself? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#779 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJzB8zESOQaEe7mExXNymkBpyMdIDks5vN9PBgaJpZM4a-5T_> .

[tech4GT]

Hmm, I think we would need to make some changes to support 2 brightness modules with different inputs, since there can be only one value of options.brightness

[tech4GT]

Maybe we can allow for making the value of an input an array, then this will be passed inside options to the module and the module can then use them as it needs?

// for example
module.exports = require('../../util/createMetaModule.js')(
  function mapFunction(options) {

    // return steps with options:
    return [
      { 'name': 'brightness', 'options': { 'brightness': options.brightness[0] } },
      { 'name': 'brightness', 'options': { 'brightness': options.brightness[1] } }
    ];
  }, {
    infoJson: require('./info.json')
  }
)[0];

[jywarren]

Well, let's just document that both will get the same values unless specified in the map function? We can demonstrate this with a test.

…

On Sat, Feb 16, 2019, 4:57 AM Varun Gupta ***@***.*** wrote: Maybe we can allow for making the value of an input an array, then this will be passed inside options to the module and the module can then use them as it needs? // for examplemodule.exports = require('../../util/createMetaModule.js')( function mapFunction(options) { // return steps with options: return [ { 'name': 'brightness', 'options': { 'brightness': options.brightness[0] } }, { 'name': 'brightness', 'options': { 'brightness': options.brightness[1] } } ]; }, { infoJson: require('./info.json') } )[0]; — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#779 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ6H1Gi24t07dTIycSDnO2wjfBNJtks5vN9YCgaJpZM4a-5T_> .

[tech4GT]

@jywarren Actually what I am suggesting is that the infoJson of the module can define the attribute as an array and give it's default value as an array, since all the generator function is doing is getting the values from infoJson, it will work.
What we would have do do here is add support for array type inputs in sequencer UI and this should be ready to go as it is!

[tech4GT]

I'll demonstrate with an example, so I know my meta-module calls the brightness module 2 times, I can define it as an array in my infoJSON

{
 "inputs":
          {
"brightness": {
            "type": "array",
            "desc": "",
            "default": ["2","3"],
        }
          }
}

[tech4GT]

I'll get this array in my options argument of the map function and I can use it as required, so we don't need that boilerplate code still.
One more thing we can Allow this like a string too!
We can then split the input on space if the type is array, this will simplify things even further.
I'll go ahead and implement this right now, maybe It'll clearer to see it in action! :)

[jywarren]

Hmm, it makes sense but it's a bit strange to use array order in one module to set a param in the order of steps... You know? Like what if there are lots of mixed modules inside, it'd be really tough to keep track. Also what does the UI display for the inputs for an array of values? I like the idea of infojson being used to set the defaults (although the map function would still determine which external values are assigned to internal ones most clearly). But shouldn't it set the defaults of the externally exposed parameters only, so the module doesn't expose any of it's internals to the outside? The defaults of the internal modules would (if they're different than their already existing defaults) need to be set internally, so the map function still makes sense there, I think. How does this sound?

…

On Sat, Feb 16, 2019, 5:25 AM Varun Gupta ***@***.*** wrote: I'll demonstrate with an example, so I know my meta-module calls the brightness module 2 times, I can define it as an array in my infoJSON { "inputs": {"brightness": { "type": "array", "desc": "", "default": ["2","3"], } } } — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#779 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ9x7MtyYPB9WYjWqhSekqf80RGMrks5vN9yUgaJpZM4a-5T_> .

[tech4GT]

Hmm, I think you are right, So let's keep the map function to allow customized mapping but still set the defaults in the create function itself, so the module just gets a list of values and can consume them whichever way it chooses to.
The UI can just display a string input which we can split on spaces.
How does that sound?

[jywarren]

Awesome. We may come up with better ideas soon but this is a good starting point. Thanks!

…

On Sat, Feb 16, 2019, 5:37 AM Varun Gupta ***@***.*** wrote: Hmm, I think you are right, So let's keep the map function to allow customized mapping but still set the defaults in the create function itself, so the module just gets a list of values and can consume them whichever way it chooses to. The UI can just display a string input which we can split on spaces. How does that sound? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#779 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJwuu3BnLZjO_ViNiJ0bRslBeTaeYks5vN99TgaJpZM4a-5T_> .

[tech4GT]

@jywarren On second though maybe we should leave it up to the module how it wants to consume multi-value inputs like you originally suggested. We can add a section about this in the documentation with an example using space separated values like we do for convolution, but basically the input type will be string.(They can use whatever delimiter they want).
How does that sound?

[jywarren]

This sounds great. Thanks Varun!

…

On Sat, Feb 16, 2019, 10:18 AM Varun Gupta ***@***.*** wrote: @jywarren <https://github.com/jywarren> On second though maybe we should leave it up to the module how it wants to consume multi-value inputs like you originally suggested. We can add a section about this in the documentation with an example using space separated values like we do for convolution, but basically the input type will be string.(They can use whatever delimiter they want). How does that sound? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#779 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ43xsf0z8DoDXctLBVKJovcZcBmxks5vOC8ugaJpZM4a-5T_> .

[tech4GT]

Okay updating the docs now! We can then merge yay! 🎉

[jywarren]

🎉🎉🎉

…

On Sat, Feb 16, 2019, 10:21 AM Varun Gupta ***@***.*** wrote: Okay updating the docs now! We can then merge yay! 🎉 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#779 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ85RRNgosZPoTyulVW0wo8gOBTguks5vODACgaJpZM4a-5T_> .

[tech4GT]

@jywarren Please have a look at the docs, if we need to add something.
Let's also update a major version after this update since it breaks the old metaModule API

[tech4GT]

Okay the test pass, docs are ready. Merging this in now! Awesome!