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

Added a 'Domain' in Chaining #53

Merged
merged 7 commits into from
Jul 10, 2017
Merged

Conversation

ccpandhare
Copy link
Collaborator

@ccpandhare ccpandhare commented Jul 10, 2017

What this PR contains

For simpler usage of Image Sequencer, we allowed chaining in methods.
So there arose am ambiguity as to what the following line of code should mean:

sequencer.loadImage(PATH).addSteps(STEPS).run();

Before

Before this pull request, what this code did was:

  • Create a new Image from the given PATH
  • give it the name "default"
  • addSteps STEPS to ALL images.
  • run the sequencer.

In short, addSteps and other modules, according to the definition in README.md, used to by default affect ALL images. This was sensible before we allowed chaining.

After

But now, what I propose in this PR is that, methods chained after loadImage() must only affect the newly loaded images. So the above piece of code will now mean:

  • Create a new Image from PATH
  • give it a random name like "image1"
  • addSteps STEPS to the newly loaded image.
  • run the sequencer on THAT image.

As you can see, the code is more intuitive now. Also I have added Automatic Image Naming like "image1", "image2", "image3", etc.

I have also forbidden chaining any methods after run() as the method will be called before run completes.

@ccpandhare
Copy link
Collaborator Author

This resolves #48

@ccpandhare ccpandhare mentioned this pull request Jul 10, 2017
@ccpandhare
Copy link
Collaborator Author

I have tried to explain this in detail. If I have not explained something in a proper manner, please let me know :-)

@jywarren
Copy link
Member

jywarren commented Jul 10, 2017 via email

@ccpandhare
Copy link
Collaborator Author

Yes that's right! I have added this to the README.md under a new section "Chaining".

@jywarren
Copy link
Member

jywarren commented Jul 10, 2017 via email

@ccpandhare
Copy link
Collaborator Author

Merging!

@ccpandhare ccpandhare merged commit fdf3d14 into publiclab:master Jul 10, 2017
@ccpandhare ccpandhare mentioned this pull request Jul 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants