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

New ability to create a Jupyter/Ipython notebook instead of the analyze script #639

Merged
merged 3 commits into from Feb 15, 2017

Conversation

Lnaden
Copy link
Contributor

@Lnaden Lnaden commented Feb 14, 2017

This does not auto-render, but it does auto generate based on the settings.

This commit includes:

  • Template Jupyter Notebook used to construct new ones
  • Example notebook fully rendered with real data (can be removed from repo once approval is done)
  • Updated version to 0.15.1
  • Automatically include the template ipython notebook in the installed repo
  • Template is fully load even if YANK is installed as .egg

…ze script.

This does not auto-render, but it does auto generate based on the settings.

This commit includes:
* Template Jupyter Notebook used to construct new ones
* Example notebook fully rendered with real data (can be removed from repo once approval is done)
* Updated version to 0.15.1
* Automatically include the template ipython notebook in the installed repo
* Template is fully load even if YANK is installed as .egg
Copy link
Contributor

@andrrizzi andrrizzi left a comment

Choose a reason for hiding this comment

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

So cool! I could only spot few typos in the template/example notebook (I can't point them in the diff):

  • manditory -> mandatory
  • therefor -> therefore
  • yeild -> yield
  • nubmer -> number

In the future, we could make it even cleaner by encapsulating some of the code in the notebook in functions in a separate report.py module.

@andrrizzi
Copy link
Contributor

We could also pre-render the notebook if we let the user specify the mandatory settings through the CLI with something like this.

@Lnaden
Copy link
Contributor Author

Lnaden commented Feb 14, 2017

pre-rendering is something I wanted to have also, but figured it could wait until we further detailed what we wanted in this report. Besides the matplotlib-fu to get the pictures of what analyze is doing, are there other features you can think of that we want in the report on this initial commit?

@andrrizzi
Copy link
Contributor

are there other features you can think of that we want in the report on this initial commit?

Not that I can think of.

@jchodera
Copy link
Member

This is great!

I'd move as much of the analysis code to Yank/analysis.py as possible in order to have the notebook really focus on the data presentation rather than the data analysis.

@Lnaden
Copy link
Contributor Author

Lnaden commented Feb 15, 2017

I'm not quite sure I follow. All of the data crunching is handled by the analyze.py module. Unless you are referring to the last code cell where its running through a subset of commands the analyze() function does. I can refactor the analyze.py script to avoid some of that code duplicate if that is what you are referring to.

The rest of the code is matplitlib-fu and formatting.

@jchodera
Copy link
Member

Plotting and formatting code can also be included into analyze.py (or even notebook.py) to minimize the code needed in the notebook, couldn't it?

@Lnaden
Copy link
Contributor Author

Lnaden commented Feb 15, 2017

That is a good point. I'll work to bring all that code into a .py file somewhere. to minimize what is in the notebook.

@Lnaden
Copy link
Contributor Author

Lnaden commented Feb 15, 2017

@jchodera I have split all of the notebook functions into a new module class which handles all the formatting. Is this more what you had in mind for the notebook?

I also think that we can make improvements on this from here if this initial report looks good.

@jchodera
Copy link
Member

@jchodera I have split all of the notebook functions into a new module class which handles all the formatting. Is this more what you had in mind for the notebook?

This is perfect! It looks much cleaner now.

I also think that we can make improvements on this from here if this initial report looks good.

Agreed!

@Lnaden
Copy link
Contributor Author

Lnaden commented Feb 15, 2017

Great! I also got some input from the people here for improvements we can make with additional details. For now though, Merging!

@Lnaden Lnaden merged commit aa51a42 into master Feb 15, 2017
@Lnaden Lnaden deleted the health-report branch February 24, 2017 18:30
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.

None yet

3 participants