-
Notifications
You must be signed in to change notification settings - Fork 387
Update tutorials to match with the new way of running experiments #802
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
Conversation
Pull Request Test Coverage Report for Build 4922
💛 - Coveralls |
"\n", | ||
"# create the experiment object\n", | ||
"exp = Experiment(env)\n", | ||
"flow_params = dict(\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add the descriptions to each parameter the way we do in the examples?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I removed the descriptions for 2 reasons: 1- It would make the tutorial kind of long. I realized most of code snippets in the tutorials are short, not to distract the reader. 2- Those parameters are all explained before in the tutorial.
@AboudyKreidieh I am open to discussing. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good to me. So long as we agree it makes the tutorials clearer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Minor nit, open to discussing if you think it's not worth it
@AboudyKreidieh can you please check my last commit and confirm it is correct? |
Pull request information
Description
Update tutorials to match with the new way of running experiments. This is one of the tasks of #720 that was excluded from the PR as that PR was getting huge.