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.

Sign up for GitHub

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

Jump to bottom

Extra explanation for Experiment and database in 15_minutes notebook #2838

Merged
merged 8 commits into from
Mar 31, 2021
Merged

Extra explanation for Experiment and database in 15_minutes notebook #2838

merged 8 commits into from
Mar 31, 2021

Conversation

FarBo
Copy link
Contributor

@FarBo FarBo commented Mar 22, 2021

This PR:

  1. Adds some extra explanation exphasasing on Experiment creation before Measurement.
  2. Mentioning config file change is temporary for the current kernel
  3. Adds explanation about running Experiment creates or load database.

@codecov
Copy link

codecov bot commented Mar 22, 2021 •
edited
Loading

Codecov Report

Merging #2838 (42b927d) into master (f116f85) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2838   +/-   ##
=======================================
  Coverage   65.35%   65.35%           
=======================================
  Files         205      205           
  Lines       27959    27959           
=======================================
  Hits        18273    18273           
  Misses       9686     9686

@FarBo FarBo closed this Mar 23, 2021
@FarBo FarBo reopened this Mar 23, 2021
@FarBo
Copy link
Contributor Author

FarBo commented Mar 23, 2021

I will perform a bit restructuring in this notebook as I think the current flow can be improved a bit. Then, It could be easier to convey what I want to add in this PR.
So please do not merge it until then.

@FarBo
Copy link
Contributor Author

FarBo commented Mar 30, 2021

@astafan8
You may look at the new changes and see if you see any improvement :)

astafan8
astafan8 approved these changes Mar 30, 2021
View reviewed changes
Copy link
Contributor

@astafan8 astafan8 left a comment

Choose a reason for hiding this comment

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

It's much better, thanks a lot @FarBo ! (just one little comment)

@FarBo
Copy link
Contributor Author

FarBo commented Mar 31, 2021

@jenshnielsen

Should we merge this?

@FarBo FarBo enabled auto-merge March 31, 2021 11:06
@FarBo FarBo merged commit fc07470 into microsoft:master Mar 31, 2021
@FarBo FarBo mentioned this pull request Mar 31, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jenshnielsen jenshnielsen jenshnielsen approved these changes

@astafan8 astafan8 astafan8 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@FarBo @jenshnielsen @astafan8