Add configurable parameter for blend module to chose image #424

JKusio · 2018-10-18T21:14:43Z

#421
Uploading again.
Last time there was a problem with Travis Ci.

JKusio · 2018-10-18T21:53:40Z

toCliString() returns the CLI command for the sequence ✖ works correctly ------------------ operator: deepEqual expected: |- 'sequencer -i [PATH] -s "channel channel channel channel channel invert blend" -d \'{"channel":"green"}\'' actual: |- 'sequencer -i [PATH] -s "channel channel channel channel channel invert blend" -d \'{"channel":"green","offset":-2}\'' stack: |- Error: works correctly at Test.assert [as _assert] (http://localhost:35925/bundle.js:47964:54) at Test.bound [as _assert] (http://localhost:35925/bundle.js:47816:32) at Test.tapeDeepEqual (http://localhost:35925/bundle.js:48161:10) at Test.bound [as deepEqual] (http://localhost:35925/bundle.js:47816:32) at Test.<anonymous> (http://localhost:35925/bundle.js:53118:5) at Test.bound [as _cb] (http://localhost:35925/bundle.js:47816:32) at Test.run (http://localhost:35925/bundle.js:47835:10) at Test.bound [as run] (http://localhost:35925/bundle.js:47816:32) at next (http://localhost:35925/bundle.js:47613:15) at onNextTick (http://localhost:35925/bundle.js:48730:12)

This seems to be a problem

JKusio · 2018-10-19T09:35:09Z

/home/travis/build/publiclab/image-sequencer/src/Run.js:79 var prevstep = ref.images[image].steps[json_q[image] - 1]; ^ TypeError: Cannot read property 'steps' of undefined at filter (/home/travis/build/publiclab/image-sequencer/src/Run.js:79:39) at Run (/home/travis/build/publiclab/image-sequencer/src/Run.js:90:16) at Object.run (/home/travis/build/publiclab/image-sequencer/src/ImageSequencer.js:160:21) at Test.<anonymous> (/home/travis/build/publiclab/image-sequencer/test/modules/image-manip.js:23:13) at Test.bound [as _cb] (/home/travis/build/publiclab/image-sequencer/node_modules/tape/lib/test.js:76:32) at Test.run (/home/travis/build/publiclab/image-sequencer/node_modules/tape/lib/test.js:95:10) at Test.bound [as run] (/home/travis/build/publiclab/image-sequencer/node_modules/tape/lib/test.js:76:32) at Immediate.next (/home/travis/build/publiclab/image-sequencer/node_modules/tape/lib/results.js:71:15) at runCallback (timers.js:672:20) at tryOnImmediate (timers.js:645:5)

This is the second error.

Is it becoue my code messed up with something?
I'm new to this 👍

JKusio · 2018-10-20T13:16:08Z

I figured out what was wrong!
Can I get a review of my code 👍? @tech4GT @jywarren

jywarren

This is super, thanks for your hard work! I made a suggestion for a specific test to demonstrate this functionality. I hope it makes sense! Great problem solving!

jywarren · 2018-10-20T22:38:18Z

test/modules/image-sequencer.js

@@ -184,7 +184,7 @@ test('getStep(offset) returns the step at offset distance relative to current st
 });

 test('toCliString() returns the CLI command for the sequence', function(t) {
-  t.deepEqual(sequencer.toCliString(), `sequencer -i [PATH] -s "channel channel channel channel channel invert blend" -d '{"channel":"green"}'`, "works correctly");
+  t.deepEqual(sequencer.toCliString(), `sequencer -i [PATH] -s "channel channel channel channel channel invert blend" -d '{"channel":"green","offset":-2}'`, "works correctly");


Here, perhaps we should introduce a new test for offsets, and we could do a test demonstrating that the default offset and -3 don't return the same result. You can run a sequence that inverts repeatedly a few times, then does blend, and this kind of test should work. You could say -3 isn't the same as -2, but -4 is the same, you know? And create a new test specifically for this functionality.

I'll think about it and try to add new test soon.

Is there some kind of documentation on how to do tests? It's time consuming by doing a lot of trials, espiecially when I can't run it locally, and each time I have to run in on Travis Ci, becouse there are some errors on the windows.

ah, have you had trouble running tests locally? I think you should be able to with npm test -- but if you did something like:

t.notDeepEqual(sequencer.toCliString(), `sequencer -i [PATH] -s "invert invert blend" -d '{"channel":"green","offset":-2}'`, "works correctly");

Docs are here: https://github.com/substack/tape

Tutorial: https://ponyfoo.com/articles/testing-javascript-modules-with-tape

But you're right, we should add these to a Testing section of the README -- would you be interested in adding a section like that? Otherwise I can open an issue for someone new to try it out.

Thanks @KusioDev !! This is going to be awesome.

I wanted to write test that invert twice first, and then blend.
It saves the first output to variable, change the offset to -3, and re-run drawing the blend.
However, when I run sequencer.run() after changing the offset, the outuput is set to undefined, and so the test is not correct, becouse I can't read property of .output.src if output is not set.

jywarren · 2018-10-23T17:08:51Z

Hmm, have you pushed up a commit with this in it? That way we can take a look and try running it ourselves?

…

On Tue, Oct 23, 2018 at 12:24 PM KusioDev ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In test/modules/image-sequencer.js <#424 (comment)> : > @@ -184,7 +184,7 @@ test('getStep(offset) returns the step at offset distance relative to current st }); test('toCliString() returns the CLI command for the sequence', function(t) { - t.deepEqual(sequencer.toCliString(), `sequencer -i [PATH] -s "channel channel channel channel channel invert blend" -d '{"channel":"green"}'`, "works correctly"); + t.deepEqual(sequencer.toCliString(), `sequencer -i [PATH] -s "channel channel channel channel channel invert blend" -d '{"channel":"green","offset":-2}'`, "works correctly"); I wanted to write test that invert twice first, and then blend. It saves the first output to variable, change the offset to -3, and re-run drawing the blend. However, when I run sequencer.run() after changing the offset, the outuput is set to undefined, and so the test is not correct, becouse I can't read property of .output.src if output is not set. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#424 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ3grl7-Q5aXYjDzmrhBlG5YjGzw9ks5un0KcgaJpZM4XvP89> .

JKusio · 2018-10-23T17:13:41Z

I did it on different branch so each update won't be send here. I'll send the new version in a minute.

jywarren · 2018-10-23T17:39:49Z

Hm, interesting, it does seem to be passing?

JKusio · 2018-10-23T17:56:57Z

Yes, but it doesnt even start the t.notEuqal().
Do you have discord or something like that, so I could discuss it with you?

JKusio · 2018-10-23T18:07:15Z

It should be working right now 👍 Can you check if everything is ok 😄

jywarren · 2018-10-23T19:03:37Z

This looks awesome. Thanks so much!!!!!

jywarren · 2018-11-11T17:48:06Z

Hi, @KusioDev -- this great addition would be good on the "Overlay" module too, but it doesn't seem to be working there. Would you be interested in trying to debug this? Based on your work in this PR i think it should be pretty easy for you. See it malfunctioning here in the final step:

http://sequencer.publiclab.org/examples/#steps=crop{x:0|y:0|w:611|h:479},gradient{},colormap{colormap:stretched},crop{x:0|y:0|w:611|h:60},resize{resize:25%25},overlay{x:5|y:5|offset:-5}

JKusio · 2018-11-11T22:59:09Z

Ok, I'll take care of it 😄

jywarren · 2018-11-12T00:13:50Z

Oh that's awesome. Thanks!!

Add configurable parameter for blend module to chose image

5c5e7b9

Made minor changes

5c86743

Edit test to work with new parameter

62ed7ef

jywarren reviewed Oct 20, 2018

View reviewed changes

kusiodev added 2 commits October 23, 2018 19:20

blend offset test added

24ba2ae

blend offset test added

b21884c

fixed blend offset test

3ee1a65

jywarren merged commit e5e372d into publiclab:main Oct 23, 2018

JKusio mentioned this pull request Nov 12, 2018

fixed offset option #468

Closed

grvsachdeva mentioned this pull request Jan 4, 2019

configurable parameters for blend module image number #421

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add configurable parameter for blend module to chose image #424

Add configurable parameter for blend module to chose image #424

JKusio commented Oct 18, 2018

JKusio commented Oct 18, 2018 •

edited

Loading

JKusio commented Oct 19, 2018 •

edited

Loading

JKusio commented Oct 20, 2018

jywarren left a comment

jywarren Oct 20, 2018

JKusio Oct 20, 2018

JKusio Oct 22, 2018

jywarren Oct 23, 2018

JKusio Oct 23, 2018

jywarren commented Oct 23, 2018 via email

JKusio commented Oct 23, 2018

jywarren commented Oct 23, 2018

JKusio commented Oct 23, 2018

JKusio commented Oct 23, 2018

jywarren commented Oct 23, 2018

jywarren commented Nov 11, 2018

JKusio commented Nov 11, 2018

jywarren commented Nov 12, 2018

Add configurable parameter for blend module to chose image #424

Add configurable parameter for blend module to chose image #424

Conversation

JKusio commented Oct 18, 2018

JKusio commented Oct 18, 2018 • edited Loading

JKusio commented Oct 19, 2018 • edited Loading

JKusio commented Oct 20, 2018

jywarren left a comment

Choose a reason for hiding this comment

jywarren Oct 20, 2018

Choose a reason for hiding this comment

JKusio Oct 20, 2018

Choose a reason for hiding this comment

JKusio Oct 22, 2018

Choose a reason for hiding this comment

jywarren Oct 23, 2018

Choose a reason for hiding this comment

JKusio Oct 23, 2018

Choose a reason for hiding this comment

jywarren commented Oct 23, 2018 via email

JKusio commented Oct 23, 2018

jywarren commented Oct 23, 2018

JKusio commented Oct 23, 2018

JKusio commented Oct 23, 2018

jywarren commented Oct 23, 2018

jywarren commented Nov 11, 2018

JKusio commented Nov 11, 2018

jywarren commented Nov 12, 2018

JKusio commented Oct 18, 2018 •

edited

Loading

JKusio commented Oct 19, 2018 •

edited

Loading