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

nbconvert: Transformer tests #3914

Merged
merged 21 commits into from Aug 12, 2013
Merged

nbconvert: Transformer tests #3914

merged 21 commits into from Aug 12, 2013

Conversation

jdfreder
Copy link
Member

@jdfreder jdfreder commented Aug 5, 2013

[not for 1.0]

Adds tests for transformers.

@@ -79,7 +80,7 @@ def transform_cell(self, cell, resources, cell_index):
else:
data = data.encode("UTF-8")

#Build a figure name
#Build a output name
Copy link
Member

Choose a reason for hiding this comment

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

s/a/an

@jdfreder
Copy link
Member Author

jdfreder commented Aug 6, 2013

@minrk , second pass?

@jdfreder jdfreder mentioned this pull request Aug 6, 2013
6 tasks
@jdfreder
Copy link
Member Author

jdfreder commented Aug 6, 2013

This contains a couple fixes for the existing transformers, should I open a separate PR targeting 1.0 to merge those fixes?

@minrk minrk mentioned this pull request Aug 6, 2013
minrk added a commit that referenced this pull request Aug 7, 2013
nbconvert: Backport fixes

cherry-picked from #3914 and #3923, which include tests that will not be merged into 1.0.
@jdfreder
Copy link
Member Author

bump 🚗

@Carreau
Copy link
Member

Carreau commented Aug 12, 2013

Looks good to me. we have a few month to refine.
Merging in a day if no objections.

"""Build an empty resources dictionary."""

res = ResourcesDict()
res['metadata'] = ResourcesDict()
Copy link
Member

Choose a reason for hiding this comment

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

Does metadata need to be a ResourceDict ? Or should we also test the fact that when it not a resource dict thing works ? (you probably know nbconvert more than me now)

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, just so we can do conversions if something doesn't exist (like notebook name). Otherwise Jinja will crash when parsing if exporting without a notebook name.

@Carreau
Copy link
Member

Carreau commented Aug 12, 2013

After carefull re-reading I have some concern of the use of ResourceDict as then test might actually be testing that attributes are (read or written) as we want to test that they are (written)

def build_notebook(self):
"""Build a notebook in memory for use with transformer tests"""

outputs = [nbformat.new_output(output_type="stream", stream="stdout", output_text="a"),
Copy link
Member

Choose a reason for hiding this comment

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

You could also use IPython.nbformat.v3.tests.nbexamples for an example notebook

ellisonbg added a commit that referenced this pull request Aug 12, 2013
@ellisonbg ellisonbg merged commit 05c7b0a into ipython:master Aug 12, 2013
@jdfreder jdfreder deleted the trans_tests branch March 10, 2014 18:44
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
nbconvert: Backport fixes

cherry-picked from ipython#3914 and ipython#3923, which include tests that will not be merged into 1.0.
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
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

5 participants