Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Opaque meta module + test - colorbar #764

Merged
merged 6 commits into from
Feb 13, 2019
Merged

Conversation

tech4GT
Copy link
Member

@tech4GT tech4GT commented Feb 12, 2019

(replaces #762) -- copying in description from there:

Re #315 -- an attempt to refactor the colorbar module to be a fully opaque meta module - that is, a module which:

  1. contains other modules
  2. acts like a normal module in every way
  3. contains an independent sequencer in order to pass an input through a sequence and produce an output

If this works (and it's not running yet -- I'm stuck for some reason because draw() is not executing... help!) then we may be able to create a generator for these, which is to say, a way to automate the creation of opaque meta modules given:

  1. a JSON object or string of steps
  2. a function which maps the outer module options to the options objects of the internal steps (or falling back to the defaults if mapping is not provided)
  3. a set of overriding defaults for internal steps (which could be done via the mapping function too)

@publiclab/is-reviewers take a look here, i got inspired and tried it out, but need a bit of help to debug my work!

@tech4GT
Copy link
Member Author

tech4GT commented Feb 12, 2019

@jywarren Actually there is no way of testing the options.inBrowser thing since we run the tests in node and inBrowser is false there anyway, haha.
Anyway I have fixed the condition and I don't think we need a test here, I tried all cases manually though. :)

Also I think we should resolve circular dependencies while dynamically loading a module into sequencer, thoughts?

@tech4GT
Copy link
Member Author

tech4GT commented Feb 12, 2019

Long term:
@jywarren I just had an Idea!! If we think scale we can bootstrap this sequencer inside sequencer idea to multiple image use cases like map-knitter, where we can initialize one instance of Sequencer for each image and run all of them as independent processes!!
Wouldn't this parallelize our workflow?? Eager to hear your thoughts!! 😃

@jywarren
Copy link
Member

(re:#762)

Re parallelize, EXACTLY!!!

Re test, can we just do a looks_like comparison with a visually confirmed output image? I just think any test for colorbar would be good. Let's confirm this can be added as a step, it produces an output, and that output looks like a visually confirmed output image.

Re circular, I saw your comment and we can try that after this, yeah. Great!

@jywarren
Copy link
Member

Also, what went wrong with this Travis build? Thanks!!

@tech4GT tech4GT closed this Feb 13, 2019
@tech4GT tech4GT reopened this Feb 13, 2019
@tech4GT
Copy link
Member Author

tech4GT commented Feb 13, 2019

@jywarren Writing up the test and docs for this now!
Let's make the generator in another PR, also please do take a look at the optimizations pr!
Thanks!

var internalSequencer = ImageSequencer({ inBrowser: false });
console.log('colorbar setup is', internalSequencer);
// ui: false prevents internal logs
var internalSequencer = ImageSequencer({ inBrowser: false, ui: false });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should document this too!

@jywarren jywarren changed the title Fix meta op Opaque meta module + test - colorbar Feb 13, 2019
@jywarren jywarren merged commit fa30c1b into publiclab:main Feb 13, 2019
@jywarren
Copy link
Member

this is great, thanks @tech4GT !!!

@jywarren
Copy link
Member

OK - now see: #765!

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

Successfully merging this pull request may close these issues.

2 participants