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

Made Pipelines user guide consistent and clearer #735

Merged
merged 1 commit into from
Nov 1, 2019

Conversation

jbednar
Copy link
Member

@jbednar jbednar commented Oct 31, 2019

Main changes:

  • The first section was written as if there was only one output, but the code had two; looks like the markdown wasn't updated when the code was
  • Removed stray , ready_parameter='ready', auto_advance=True in early example; looks like something meant for the last example
  • Clarified hierarchical arrangement of .layout
  • Cleaned up descriptions and narrative flow

I opened some issues found while reviewing this (#731, #732, #733, #734). When those are fixed, the text in this notebook will need to be updated accordingly, so this PR should probably be merged before addressing any of those (or updated each time).

One additional issue is that I wasn't sure why .init() needed to be called or precisely when it should be called; isn't there some way that it can be called automatically? It's a confusing, stateful thing...

@jbednar jbednar requested a review from philippjfr October 31, 2019 22:43
@philippjfr
Copy link
Member

The init issue can probably be solved thinking about it. The issue was that it wasn't obvious how to set up the graph when you declare a simple linear pipeline without calling define_graph, but if we simply build up the linear pipeline as stages are added and then override if define_graph is called there is no problem.

@jbednar
Copy link
Member Author

jbednar commented Oct 31, 2019

That sounds right from what I remember of that code, thanks. I've opened #736 as a request to address that.

@philippjfr
Copy link
Member

Changes look good thanks!

@philippjfr philippjfr merged commit bf47e07 into master Nov 1, 2019
@philippjfr philippjfr deleted the pipeline_clarity branch January 23, 2020 02:02
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