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

provide some example usage and description #1

Merged
merged 6 commits into from
Jul 6, 2017
Merged

Conversation

rgbkrk
Copy link
Member

@rgbkrk rgbkrk commented Jul 6, 2017

Kicking off a README that fleshes the ideas out a bit more.

@rgbkrk rgbkrk requested a review from ewmassey July 6, 2017 18:49
README.rst Outdated

⚠️ Papermill is a Work-in-progress and this README describes the intended state. 😁
Stepping away from the hyperbole, the goals are to assist in:
Copy link
Member

Choose a reason for hiding this comment

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

the goals our goals for Papermill include simplifying and streamlining

Copy link
Member Author

Choose a reason for hiding this comment

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

excellent, thank you

README.rst Outdated
@@ -1,10 +1,44 @@
|Logo|
=========

Papermill is map reduce for jupyter notebooks.
Copy link
Member

Choose a reason for hiding this comment

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

Jupyter


Usage
-----

Copy link
Member

Choose a reason for hiding this comment

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

Python 2.7+ or 3.4+?

Copy link
Member Author

Choose a reason for hiding this comment

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

Caught me, I was under the assumption that this would all work in both. I'll make sure to set up tox here shortly. 😄

Copy link
Member

Choose a reason for hiding this comment

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

Probably will work with both. Do you want both?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes

README.rst Outdated

nb = pm.read_notebook("output.ipynb")

Creating a parametrized notebook, recording metrics::
Copy link
Member

Choose a reason for hiding this comment

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

...notebook and recording...


rmse = metrics.mean_squared_error(...)
pm.record_value("rmse", rmse)
plot() # Tag this cell as "results" for extraction later
Copy link
Member

Choose a reason for hiding this comment

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

I would make the quotes consistent with line 52. Single or double.

Copy link
Member Author

Choose a reason for hiding this comment

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

Somehow missed this one, whoops.

Copy link
Member

@willingc willingc left a comment

Choose a reason for hiding this comment

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

A few small things...

@rgbkrk
Copy link
Member Author

rgbkrk commented Jul 6, 2017

Whoa thanks @willingc!

README.rst Outdated
pm.execute_notebook(
notebook="template.ipynb",
output="output.ipynb",
params=dict(alpha=0.1, l1_ratio=0.001)
Copy link
Member

Choose a reason for hiding this comment

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

Looking at the rendered view now, it would be nice to make l1_ratio more readable since the l and 1 are confusing side by side.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's so true. This is an artifact of the model being built underneath relying on linear_model.ElasticNet from scikit learn which has an l1_ratio parameter. Since this is for the readme I'll make it just ratio since it's for illustration purposes.

@rgbkrk rgbkrk merged commit b017ed7 into master Jul 6, 2017
@rgbkrk rgbkrk deleted the provide-intended-usage branch July 6, 2017 21:05
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

2 participants