Meta modules #308

tech4GT · 2018-07-19T17:58:38Z

@jywarren meta modules🎉

Signed-off-by: tech4GT <[email protected]>

tech4GT · 2018-07-19T18:07:16Z

@jywarren As of now meta-modules do not support any meta-data apart from a name, which means no description and their options are inherited from the sequence they are created, also I have added a button in the ui from which the current sequence can be saved as meta module, they are stored in the local-storage of the browser and inside a json fille in node

jywarren · 2018-07-19T21:51:30Z

Wow, very cool! Can you walk me through how they work, by writing some docs? Imagine (haha) that i'm a developer first learning about the idea, and I can help by reading through the docs and code to see if it makes sense.

Very exciting!!!

tech4GT · 2018-07-20T08:48:33Z

@jywarren I'll add some comments and docs plus a test for this

Signed-off-by: tech4GT <[email protected]>

jywarren · 2018-07-20T16:47:52Z

README.md

@@ -461,6 +461,13 @@ sequencer.insertSteps({
 ```
 return value: **`sequencer`** (To allow method chaining)

+## Meta Modules
+
+Image sequencer supports saving a sequence of modules and their associated settings(a stringified sequence) as a `meta-module`. These meta-modules are saved in the local storage inside the browser and inside a json file in node.js. Meta-modules can be saved in node context using the CLI option


needs a space here, small typo! and capitalize Image Sequencer? but looking great!

maybe and their associated settings in a simple string syntax, and then wrapping them up as a new module -- a "meta module" made of other modules. These meta-modules can be saved in ... or inside a json file.

jywarren · 2018-07-20T16:50:13Z

README.md

+```shell
+--save-as-meta-module "name stringified-sequence"
+```
+Sequencer supports a function `sequencer.saveMetaModule(name,sequenceString)` which saves the given string as a meta module. The function `sequencer.loadModules()` reloads the modules and meta modules into `sequencer.modules` and `sequencer.metaModules`


Image Sequencer can create/instantiate a new meta module with the function sequencer.saveMetaModule(name, sequenceString), which returns a new module containing the other modules.

Maybe also mention whether meta-modules can contain other meta modules?

And tell people to submit new meta-modules they create by opening an issue - they could be imported in or listed in the readme.

jywarren

Looking great! Just a few tweaks here. Also, do you want to include one example meta-module, maybe in a subsequent PR, to link to from the docs? Or in an example, maybe? Could we have one core module be a meta module, to show an example?

tech4GT · 2018-07-20T16:59:37Z

@jywarren No meta-modules cannot contain other meta-modules and sure I'll add one example meta module, but should people contribute meta-modules?? they are just workflows so do we really want people to add them??

jywarren · 2018-07-20T17:03:13Z

well, i think we want people to share them... maybe we can make an issue for people to chime in with ones they've found useful, and we can start collecting them? Interesting - where does the code have trouble with a meta module including another meta-module? This could be a good part of the explanation in the README.

…

On Fri, Jul 20, 2018 at 1:00 PM Varun Gupta ***@***.***> wrote: @jywarren <https://github.com/jywarren> No meta-modules cannot contain other meta-modules and sure I'll add one example meta module, but should people contribute meta-modules?? they are just workflows so do we really want people to add them?? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#308 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJx1RRcA53V9a_saUoF_7oS4Dg3Zqks5uIgx6gaJpZM4VW1vP> .

tech4GT · 2018-07-20T17:08:49Z

@jywarren actually when we expand a meta-module we just find the constituent steps in core modules that is why this

tech4GT · 2018-07-20T17:09:09Z

@jywarren I'll Include this in the readme!!

jywarren · 2018-07-20T17:15:22Z

src/ImageSequencer.js

+    // Save the given sequence string as a module
+    if (options.inBrowser) {
+      // Inside the browser we save the meta-modules using the Web Storage API
+      var metaModules = JSON.parse(window.localStorage.getItem('metaModules'));


Hmm, maybe we should think about the ability to createMetaModule() separately from where to store it. For example, shouldn't we be able to do:

var cropAndBlur = sequencer.createMetaModule('cropAndBlur', 'crop,blur'); sequencer. addSteps(cropAndBlur); // although i know we've used strings for this in the past... thoughts?

So a meta module would really be like a real module? Mind-bending, i know!

Would we then essentially need to include a whole sequencer inside the meta module? so it can accept an input, and generate an output? Or is there a simpler way?

🤯

@jywarren according to current structure we can do

sequencer.saveMetaModule('cropAndBlur','crop(),blur()') sequencer.addSteps('cropAndBlur')

@jywarren we don't need that, we can just register it like a dynamic module from a string

Ah, but can't we just save it in memory for the time being, rather than in localStorage, and then think separately about reinstantiating them from other sources? I just want to think modularly here. saveMetaModule() could just generate the JSON, and then we could have separate methods for storing that in a persistent manner?

Yes, true modules that are made up of other modules!

And thank you for thinking through this with me!

@jywarren Actually the thing is that right now we are pulling the info in from the info of constituent modules so in order to have it support info for meta modules and have it load like normal modules needs fundamentally changing the way we do modules and bake in meta-modules there

Actually I think we are better off this way coz even if we try to make meta-modules like fully functional modules at some point we would have to expand them, what i mean is that at some point we would have to remove the meta-module from our steps array and we would have to push in the constituent modules before calling sequencer.run()

@jywarren This is a little hard to explain, Actually I started doing it but I realized this that we cannot make them like normal modules unless we find some way to execute the steps inside the meta-module like you said have an instance of sequencer inside of the module to execute the sequence and return the output

Signed-off-by: tech4GT <[email protected]>

tech4GT · 2018-07-21T10:55:31Z

@jywarren sorry fo doing this here as well but I have also added the CLI for saving a module installed from npm. Here is the final workflow. register modules cli option installs the dependency from npm and saves it

sequencer --register-module "invert image-sequencer-invert"
sequencer -s invert -i ...

Signed-off-by: tech4GT <[email protected]>

tech4GT · 2018-07-24T16:39:02Z

@jywarren for meta modules I think we need to make a major design descision here since we can't do both, do we want the meta module to be a light weight sequence or a complete module with it's own info.json and stuff, I say we do the lightweight sequence, what do you say?

jywarren · 2018-07-24T17:14:13Z

How much time do you think a full module system would be? I think that if we want to implement things like the colorbar system, we may need that. I know it's a big project, but it opens up a lot of possibilities for new modules once you can truly combine them into new modules, you know? What do you think?

…

On Tue, Jul 24, 2018 at 12:39 PM Varun Gupta ***@***.***> wrote: @jywarren <https://github.com/jywarren> for meta modules I think we need to make a major design descision here since we can't do both, do we want the meta module to be a light weight sequence or a complete module with it's own info.json and stuff, I say we do the lightweight sequence, what do you say? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#308 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ8fR3_5AVtzDrfLz3J_T4eGJu6j5ks5uJ02mgaJpZM4VW1vP> .

tech4GT · 2018-07-24T18:47:57Z

@jywarren I'll start working on this now!!

tech4GT · 2018-07-24T19:23:56Z

A full module system will be very powerful @jywarren No doubt!! Let's do it then!!

jywarren · 2018-07-24T20:41:57Z

Haha, i love your attitude. Great, Varun, and thank you for your excellent energy! 👏🎉👏

…

On Tue, Jul 24, 2018 at 3:24 PM Varun Gupta ***@***.***> wrote: A full module system will be very powerful @jywarren <https://github.com/jywarren> No doubt!! Let's do it then!! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#308 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ0WH06-znZXksleWkH5UbCyz7_tVks5uJ3RNgaJpZM4VW1vP> .

…nto meta-modules

tech4GT · 2018-07-25T12:59:35Z

@jywarren So what I am trying to do now is that I am building an API that exposes the steps array to the step itself inside the draw method. So a meta-module in it's draw method would expand it's steps and push them into the sequencer steps array at runtime.

tech4GT · 2018-07-25T13:00:27Z

@jywarren can you please merge in #293 there is some code in there that I would like to use here. Thanks a lot!!

tech4GT · 2018-07-25T13:10:24Z

@jywarren One major issue that I am facing in this approach is how do we save the meta-module inside the browser context. I mean in node I can just generate a directory and a file called Modules.js and be done with it but when we are inside the browser I can't think of a way to make the meta-module persist unless I save the steps as JSON and then generate the meta-modules each time the browser reloads, do you think that is OK?

Signed-off-by: tech4GT <[email protected]>

tech4GT · 2018-07-25T17:10:47Z

@jywarren We are done with this!!!!🎉🎉🎉🎉
The new implementation is exactly as you suggested and even more..

Meta Module:

Any module can be a meta-module now!! As I suggested earlier I have made an api that can expand the meta-module steps inside of the steps array. I have even made an example of this please see Ndvi-Colormap inside the modules. For the browser side I have created a function sequencer.createMetaModule(steps, info) through which meta-module can be created and added as a script so for instance somebody trying to do for ndvi and colormap this will say this

var ndvi-colormap = sequencer.createMetaModule('ndvi(),colormap()',{name: 'ndvi-colormap',description: 'blah blah'})
sequencer.loadNewModule(ndvi-colormap,'ndvi-colormap');

Saved Sequence

A user can save his/her sequence in both node(SavedSequencer.json) and browser context(localStorage), this will show up in their steps dropdown the next time so that they can quickly restore their workflow(in the future if you would like we can separate the steps and saved sequences)
So as an end user of sequencer I can add say five steps and then click the save-sequence button and then the next time in my steps dropdown i will find my saved sequence which I can click on and it will restore my progress!!

Signed-off-by: tech4GT <[email protected]>

tech4GT · 2018-07-25T18:59:13Z

This pr closes #190 and #200

Signed-off-by: tech4GT <[email protected]>

jywarren · 2018-07-27T22:36:32Z

Sorry, i didn't get to this today! Had a big due date again. I'm going to look at it tomorrow, OK? THANKS VARUN!!! 👍

tech4GT · 2018-07-29T11:14:38Z

@jywarren also I'm kind of very busy for a few days now since my college interviews are going on... I hope that's fine!

jywarren · 2018-07-30T19:45:19Z

Reading through this carefully NOW! 🎉

jywarren

I think this is a very good pr, very readable and excellently explained. I've left a few clarifying comments for documentation additions and a question about createMetaModule, but I'm really impressed here. Great work.

Last request - a test for metamodules? Could we test a metamodule against a sequence that should be identical, to demonstrate how it works? Probably in its own test file.

Then ready to merge!!!

jywarren · 2018-07-30T19:43:00Z

CONTRIBUTING.md

+
+## Meta Module
+
+IMAGE SEQUENCER supports "meta modules" -- modules made of other modules. The syntax and structure of these meta modules is very similar to standard modules. Sequencer can also genarate meta modules dynamically with the function `createMetaModule` which can be called in the following ways


This is a great description!!!

jywarren · 2018-07-30T19:43:33Z

CONTRIBUTING.md

+```js
+//index.js
+module.exports = [
+  require('./Module.js)


oops, comma needed here!

jywarren · 2018-07-30T19:44:54Z

CONTRIBUTING.md

+  {
+  "name": "meta-moduleName",
+  "description": "",
+  "length": //Integer representing number of steps in the metaModule


Is "length" required? Or is it "nice to have"? Maybe add a note about this in the docs here?

jywarren · 2018-07-30T19:46:06Z

README.md

@@ -104,8 +104,10 @@ Image Sequencer also provides a CLI for applying operations to local files. The
    -b  | --basic            | Basic mode only outputs the final image
    -o  | --output [PATH]    | Directory where output will be stored. (optional)
    -c  | --config {object} | Options for the step. (optional)
+    --save-sequence [string] | Name space separated with Stringified sequence to save


...to save as a Meta module (see full docs) ?

jywarren · 2018-07-30T19:46:28Z

README.md

@@ -104,8 +104,10 @@ Image Sequencer also provides a CLI for applying operations to local files. The
    -b  | --basic            | Basic mode only outputs the final image
    -o  | --output [PATH]    | Directory where output will be stored. (optional)
    -c  | --config {object} | Options for the step. (optional)
+    --save-sequence [string] | Name space separated with Stringified sequence to save
+    --install-module [string] | Module name space seaprated npm package name


shall we show an example? this description is a little obscure.

@jywarren including sample commands in the readme files

jywarren · 2018-07-30T19:50:21Z

examples/demo.js

-    );
+  function refreshOptions() {
+    // Load information of all modules (Name, Inputs, Outputs)
+    var modulesInfo = sequencer.modulesInfo();


Should this be the default, and be over-ridable? Or not worth it?

jywarren · 2018-07-30T19:53:46Z

index.js

+  sequencer.saveSequence(params[0], params[1]);
+} else if (program.installModule) {
+  var params = program.installModule.split(' ');
+  var spinner = Spinner("Now Installing...").start();


Oh wow. Is this tested?

@jywarren yes!!

jywarren · 2018-07-30T19:55:54Z

src/ImageSequencer.js

@@ -324,12 +345,62 @@ ImageSequencer = function ImageSequencer(options) {
    return this;
  }

+  function saveNewModule(name, path) {


Whoa, how does this work?

jywarren · 2018-07-30T19:57:22Z

src/ImageSequencer.js

+  }
+
+  function saveSequence(name, sequenceString) {
+    const sequence = stringToJSON(sequenceString);


These are so nice with this general purpose function! Nice and short and readable.

jywarren · 2018-07-30T20:01:16Z

src/modules/NdviColormap/Module.js

+ * Sample Meta Module for demonstration purpose only
+ */
+module.exports = function NdviColormapfunction() {
+    this.expandSteps([{ 'name': 'ndvi', 'options': {} }, { 'name': 'colormap', options: {} }]);


I guess I'm wondering one last thing, could we create a metal module from a json object with createMetaModule right here, alternatively? Any pros or cons to that rather than using expandSteps? Cool!

Signed-off-by: tech4GT <[email protected]>

tech4GT · 2018-08-02T09:50:47Z

@jywarren I'll summarize everything here
install-module option firstly installs the given module from npm and then calls the save-module function to save it into sequencer by modifying the modules.js file itself. this means that the CLI user has to just enter one command on the terminal and the dependency is downloaded saved and registered into sequencer and is ready to use.
sequencer.createMetaModule can create modules on the fly with the json object and that is the intended way of making meta-modules for the ui. Also what i have demonstrated is that we can create a meta module like a regular module inside a file but all we have to do inside the module if we wish to write it ourselves is call the sequencer.expandsteps at the end of it so that the constituent steps can be pushed on the the steps array. We can say that the function exapndsteps just expands the given steps and pushes them onto the the steps array.

tech4GT · 2018-08-02T09:51:36Z

And Done!! @jywarren I think that does it all!! Tested and documented🎉

Signed-off-by: tech4GT <[email protected]>

jywarren · 2018-08-04T13:27:12Z

Great, i think we are ready to merge this -- let me test it today and I'll do it.

I was waiting because i wasn't sure of one thing -- when expandSteps is run, the meta module disappears and is replaced by the sub-modules in the main sequencer? Am I right in understanding this?

If so, I want to explore a future version where they are permanently kept inside the meta module, so no external modules ever know how the meta module took an input and produced an output. I think this kind of "wrapper" meta module, that doesn't reveal its internals, is ideal for a number of reasons. Thoughts?

But it can be in a future version! If my understanding above is correct, we are all good here! Merge?

jywarren · 2018-08-04T13:28:57Z

CONTRIBUTING.md

+```
+The length is absolutely required for a meta-module, since sequencer is optimized to re-run minimum number of steps when a step is added in the UI which is 1 in the case of normal modules, if the added step is a meta-module the length of the sequence governs the number of steps to re-run.


So with meta modules that don't get expanded into multiple main sequencer steps, but keep them internal only, this requirement might go away?

@jywarren Then that meta-module would just be a normal module, so yeah no need then :D

jywarren · 2018-08-04T13:51:45Z

What do you say, Varun, shall I go ahead and merge now? Thanks!

tech4GT · 2018-08-04T13:53:52Z

@jywarren Yeah we can do that in the future update!! Let's merge this!! Awesome!!🎉

jywarren · 2018-08-04T14:07:51Z

Hooray!!!!!!!👍🏼👍🏼👍🏼

tech4GT · 2018-08-04T14:12:11Z

@jywarren Let's wait on pushing it out to production though, let's add the colorbar module first

jywarren · 2018-08-04T14:27:14Z

Ok! On Aug 4, 2018 10:12 AM, "Varun Gupta" <[email protected]> wrote: @jywarren <https://github.com/jywarren> Let's wait on pushing it out to production though, let's add the colorbar module first — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#308 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ5ItbdN4LIBK2EMnOwLNKtS0G9Uyks5uNau7gaJpZM4VW1vP> .

tech4GT added 2 commits July 19, 2018 22:10

meta-modules in node

52e21a6

Signed-off-by: tech4GT <[email protected]>

meta-modules in browser

0d09d26

Signed-off-by: tech4GT <[email protected]>

ghost assigned tech4GT Jul 19, 2018

ghost added the in-progress label Jul 19, 2018

tech4GT added 3 commits July 20, 2018 19:23

update docs

906daa5

Signed-off-by: tech4GT <[email protected]>

more doc

733cbe3

Signed-off-by: tech4GT <[email protected]>

fix typo

c3f6693

Signed-off-by: tech4GT <[email protected]>

jywarren reviewed Jul 20, 2018

View reviewed changes

jywarren requested changes Jul 20, 2018

View reviewed changes

jywarren reviewed Jul 20, 2018

View reviewed changes

tech4GT added 2 commits July 20, 2018 22:46

update docs and sample meta-module

612897d

Signed-off-by: tech4GT <[email protected]>

add dynamic modules CLI from npm

4b0a6f8

Signed-off-by: tech4GT <[email protected]>

update

767544e

Signed-off-by: tech4GT <[email protected]>

Merge branch 'master' of https://github.com/tech4gt/image-sequencer i…

44a663a

…nto meta-modules

meta-modules and sequence saving completed

93a283c

Signed-off-by: tech4GT <[email protected]>

merged with master

8b67c07

Signed-off-by: tech4GT <[email protected]>

tech4GT added ready-to-merge and removed in-progress labels Jul 25, 2018

This was referenced Jul 25, 2018

meta-modules in node #203

Closed

"Save as module" button on Dynamic module #231

Open

add docs

d9d6eb7

Signed-off-by: tech4GT <[email protected]>

ghost added the in-progress label Jul 26, 2018

jywarren requested changes Jul 30, 2018

View reviewed changes

update docs

672831c

Signed-off-by: tech4GT <[email protected]>

fixed test

0cf1ef7

Signed-off-by: tech4GT <[email protected]>

jywarren approved these changes Aug 4, 2018

View reviewed changes

jywarren reviewed Aug 4, 2018

View reviewed changes

jywarren merged commit 5ab018f into publiclab:master Aug 4, 2018

ghost removed the in-progress label Aug 4, 2018


		## Meta Module

		IMAGE SEQUENCER supports "meta modules" -- modules made of other modules. The syntax and structure of these meta modules is very similar to standard modules. Sequencer can also genarate meta modules dynamically with the function `createMetaModule` which can be called in the following ways

		```
		The length is absolutely required for a meta-module, since sequencer is optimized to re-run minimum number of steps when a step is added in the UI which is 1 in the case of normal modules, if the added step is a meta-module the length of the sequence governs the number of steps to re-run.

Meta modules #308

Meta modules #308

Conversation

tech4GT commented Jul 19, 2018

tech4GT commented Jul 19, 2018

jywarren commented Jul 19, 2018

tech4GT commented Jul 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jywarren left a comment

Choose a reason for hiding this comment

tech4GT commented Jul 20, 2018

jywarren commented Jul 20, 2018 via email

tech4GT commented Jul 20, 2018

tech4GT commented Jul 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tech4GT Jul 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tech4GT commented Jul 21, 2018 • edited Loading

tech4GT commented Jul 24, 2018

jywarren commented Jul 24, 2018 via email

tech4GT commented Jul 24, 2018

tech4GT commented Jul 24, 2018

jywarren commented Jul 24, 2018 via email

tech4GT commented Jul 25, 2018

tech4GT commented Jul 25, 2018

tech4GT commented Jul 25, 2018

tech4GT commented Jul 25, 2018

Meta Module:

Saved Sequence

tech4GT commented Jul 25, 2018

jywarren commented Jul 27, 2018

tech4GT commented Jul 29, 2018

jywarren commented Jul 30, 2018

jywarren left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tech4GT commented Aug 2, 2018

tech4GT commented Aug 2, 2018

jywarren commented Aug 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jywarren commented Aug 4, 2018

tech4GT commented Aug 4, 2018

jywarren commented Aug 4, 2018

tech4GT commented Aug 4, 2018

jywarren commented Aug 4, 2018 via email

tech4GT Jul 20, 2018 •

edited

Loading

tech4GT commented Jul 21, 2018 •

edited

Loading