asciidoc export #355

amueller · 2016-08-10T16:46:21Z

I've started on writing an asciidoc exporter.
This is helpful particularly for people that want to write books with O'Reilly, which are a few folks these days, I think.
This PR is not ready for reviews yet, but I wanted to ask

Is this something you'd like to have in nbconvert?
What kinds of tests would you like to see for adding a new output format?

takluyver · 2016-08-10T16:55:53Z

nbconvert/exporters/__init__.py

@@ -4,6 +4,7 @@
 from .templateexporter import TemplateExporter
 from .latex import LatexExporter
 from .markdown import MarkdownExporter
+from .asciidoc import ASCIIdocExporter


I think you forgot to add a new file when you committed ;-)

indeed, fixed now, I think ;)

takluyver · 2016-08-10T16:58:27Z

I think it would be OK to add an asciidoc exporter to nbconvert, assuming it doesn't add much complexity. However, if we decide not to, it should now be easy to distribute it as a separate module, using entry points to register the exporter:

http://nbconvert.readthedocs.io/en/latest/external_exporters.html#registering-a-custom-exporter-as-an-entry-point

amueller · 2016-08-10T17:26:21Z

Either way is fine for me. Adding in here would provide more exposure. I don't see a lot of complications. It's supported by pandoc, so things should be fairly simple. It's mostly tweaking the template.

amueller · 2016-08-10T17:30:34Z

nbconvert/exporters/asciidoc.py

@@ -0,0 +1,35 @@
+"""Markdown Exporter class"""


whoops I totally didn't copy & paste this

amueller · 2016-08-22T19:31:39Z

This should be good now. Suggestions for tests welcome.

minrk · 2016-08-23T13:40:50Z

I think asciidoc is appropriate to include, especially if @amueller promises to support it going forward.

amueller · 2016-08-23T13:41:50Z

Yeah I'm happy to help.

Sent from phone. Please excuse spelling and brevity.

On Aug 23, 2016 09:40, "Min RK" [email protected] wrote:

I think asciidoc is appropriate to include, especially if @amueller
https://github.com/amueller promises to support it going forward.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#355 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAbcFt4rTbBfoL-vzLPbwqHy17Zey6Uaks5qivhjgaJpZM4JhWFN
.

mpacer · 2016-08-25T03:31:09Z

Added to 5.0, I'll think more about the tests question.

@amueller what sorts of issues were you running into while debugging this? That might be a good place to start?

I'm fine shipping it by default for exposure and ease of use sake, but could you still convert this to entry points as per #352. Right now you're set up to conflict with that once that pr is merged.

amueller · 2016-08-25T17:11:21Z

@michaelpacer can you elaborate on the entry points?
That PR doesn't seem to touch any of the exporters.

One thing that I noticed when debugging was that latex was dropped (as it is when converting to html, think? or RST?)

Things were pretty straight-forward. There is a work-around for a pandoc bug in there, the rest was pretty straight-forward. I'm wondering if we want better support for internal links. Markdown has (afaik) internal links via #tags like [reference name](#reference-tag). That is currently converted to a standard link, not an internal reference in asciidoc. arguably a pandoc bug, too. I don't think these are converted into proper internal links in any of the other conversions, though (apart from html, where there is no difference between internal and external references).

takluyver · 2016-08-26T12:32:19Z

@michaelpacer - I think we'll just have to deal with some conflicts. If @amueller converts this to use entry points, then it's definitely going to conflict with #352. Sorry, I haven't had much chance to review either of these PRs recently, what with travel.

amueller · 2016-08-30T18:28:26Z

I think I need to clean this up a bit. It seems to me instead of writing many x2x functions, there should be a generic_pandoc filter that can do all pandoc conversions. Otherwise I'll also need to add a html2asciidoc one and that all seems to blow up.

mpacer · 2016-08-30T18:41:39Z

My intuition is that while the generic pandoc converter is a good idea, that should be a separate PR as it will require a more broad refactoring of the code base at which point new bugs are likely to be introduced.

amueller · 2016-08-30T18:47:53Z

So should I first create a PR for the generic pandoc filter? Or I could add a separate html2asciidoc filter here (not sure if that's the last one).

mpacer · 2016-08-30T18:50:48Z

The only thing I thought would be good to have in the previous version, would be to use entrypoints to access it in addition to using the hardcoded export_map as we were trying to govern all the exporters via entrypoints.

In fact my worry is that your asciidoc exporter won't be seen by the get_exporter function given the removal at https://github.com/jupyter/nbconvert/pull/352/files#diff-592b46ef5ddfed7d0b228485401ea2a7L183

Could you write a test that takes one of the notebooks and exports it to asciidoc? I'm pretty sure if you add that it'll fail right now.

mpacer · 2016-08-30T18:57:44Z

As for the order of the PRs, if you think you can put it together relatively quickly it shouldn't be an issue.

The changes you'll be making for that will likely be covered by existing tests, since it's modding current infrastructure rather than adding a new exporter. So I imagine it'll be faster to debug and faster to merge, and it'll probably save you the time of needing to rewrite some of this PR if you do that first.

mpacer · 2016-08-30T19:02:23Z

Also apologies for not elaborating on the entrypoints stuff before, it shouldn't be hard, just add a line in the setup.py setup_args['entrypoints'] near https://github.com/jupyter/nbconvert/pull/352/files#diff-2eeaed663bd0d25b7e608891384b7298R217 instead of using export_map.

amueller · 2016-08-30T19:14:45Z

Ok, thanks. Don't expect anything this week, but hopefully next.

mpacer · 2016-08-31T00:04:41Z

I'm going to move this to 5.1, which I anticipate won't need too many things to be released shortly after 5.0.

If we don't release 5.0 before you can cover those cases then it should definitely be in 5.0. A new default template for working directly with a publishing house's format is a nice feature to have on a x.0 version, so that would be my preference (ceteris paribus).

mpacer · 2016-10-12T00:55:54Z

Yay, #436 was merged, so this can proceed. Feel free to define a convenience method, but I ask that for consistency inside the template itself that you use the generic_pandoc function (except in those cases where you need to do some postprocessing).

I was going to do a refactoring of the generic pandoc now that it's merged. It may be possible to use that as a way to do the postprocessing rather than needing to hardcode it in a single function (See #291 #416).

@amueller Will you have a chance to work on this soon?

amueller · 2016-10-14T16:29:49Z

I'll try over the weekend, ok?
Do you have a deadline for 5.0?

Carreau · 2016-10-17T22:46:26Z

Do you have a deadline for 5.0?

I think @mpacer would like to have a beta soonish (if possible before end of october).

If it does not make it into 5.0 we can make it into 5.1 which should be sooninsh after 5.0 cause we'll probably get complaint as we often break things unexpectedly.

mpacer · 2016-10-17T22:55:50Z

Hi @amueller, I was hoping to have this ready to go (at least as a beta release) by the end of this week.

And yes I support what @Carreau said, we can put this in 5.1, if it doesn't make it into 5.0. But given that the generic pandoc filter is included it'd be nice to have with it together, illustrating its use.

mpacer · 2016-10-17T22:57:41Z

Also, I think I'm going to not touch generic pandoc and solve the rst filtering in a slightly different way.

amueller · 2016-10-17T23:00:10Z

Ok, trying to do this today.
Somehow I get 100 scikit-learn notifications a day right now, that sort of keeps me busy :-/

adding in and out prompts add display data priority to asciidoc, tweak asciidoc template add hack to work around weird pandoc behavior add support for latex blocks nicer In/Out blocks, no extra blocks for each output. add generic pandoc conversion filter, also convert html in asciidoc add missing file

amueller · 2016-10-18T00:07:02Z

Just ran this over my book and got no errors... I think that's a relatively decent test (can't check that one in, though ;) . I'll add something to the test file after dinner.

mpacer · 2016-10-18T01:04:41Z

This is looking good. Restarted the build on travis, since it looked like it failed for bad reasons.

Also, if the latex2asciidoc filter is just using the pandoc command with no intermediate layers, could you use convert_pandoc(from_format='latex', to_format='asciidoc') in the actual template?

I'm ok if you want to leave the helper there for other people in the LaTeX filter module itself.

mpacer · 2016-10-18T01:05:00Z

Also, thank's for turning this around so quickly!

amueller · 2016-10-18T03:06:40Z

I removed the latex2asciidoc (as that was kind of the point of the other PR), but left in the markdown2asciidoc for now. The bug has been fixed in pandoc master, but hasn't been released yet.

amueller · 2016-10-18T03:59:43Z

I added a test to the exporters. Was that what you had in mind? I just realized that before I kept cramming stuff into test_nbconvertapp.py which I guess is more of an integration test? Should I test there, too?

amueller · 2016-10-18T17:32:10Z

I don't understand the test failure. That's unrelated, right? Can someone retrigger on 2.7?

takluyver · 2016-10-18T19:50:35Z

It did indeed look unrelated; I've rerun it and it passed. :-)

mpacer · 2016-10-19T02:54:22Z

@amueller Apologies for the lack of quick response. I wasn't able to spend much of any time on GitHub today.

So this looks good except that the test case that you are making the exception for in the markdown2asciidoc is not currently being hit by the testing materials. Could you modify the testing file to include whatever the error condition is for that pandoc bug?

Other than that this looks good to merge.

mpacer · 2016-10-19T06:36:40Z

Made a PR at amueller#2 to fix the code coverage issue.

As soon as that's added, I'll merge.

@amueller Sorry for not contributing directly… I would have just pushed an update here, but then I would have felt uncomfortable also merging, and I wanted to be able to do that as soon as I could.

Add test case

mpacer · 2016-10-19T16:36:55Z

Thanks @amueller for all the work you put into this!

Merging

amueller · 2016-10-19T16:48:43Z

thank you for your help in getting this in :)

takluyver reviewed Aug 10, 2016
View reviewed changes

amueller reviewed Aug 10, 2016
View reviewed changes

nbconvert/exporters/asciidoc.py

@@ -0,0 +1,35 @@

"""Markdown Exporter class"""

Copy link

Contributor Author

amueller Aug 10, 2016

Choose a reason for hiding this comment

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

whoops I totally didn't copy & paste this

amueller mentioned this pull request Aug 18, 2016

Meaning of "Out" for export #365

Closed

amueller changed the title ~~Work-in-progress: asciidoc export~~ asciidoc export Aug 22, 2016

amueller force-pushed the asciidoc branch from 4f5e257 to 1f599ae Compare August 22, 2016 19:31

jni mentioned this pull request Aug 23, 2016

Consider exporting to asciidoc elegant-scipy/elegant-scipy#175

Closed

mpacer added this to the 5.0 milestone Aug 25, 2016

mpacer mentioned this pull request Aug 25, 2016

Create entrypoints #352

Merged

mpacer modified the milestones: 5.1, 5.0 Aug 31, 2016

amueller mentioned this pull request Sep 30, 2016

General pandoc filter #436

Merged

mpacer modified the milestones: 5.0, 5.1 Oct 11, 2016

amueller force-pushed the asciidoc branch from 9349963 to 9e06098 Compare October 17, 2016 23:47

amueller force-pushed the asciidoc branch from 9e06098 to 9799fa3 Compare October 17, 2016 23:53

amueller added 2 commits October 17, 2016 20:02

proper CamelCase, add to setup.py

2e48d07

undo all the crud from export

dfd7dc0

fix docstrings.

8e6ff4f

remove latex2asciidoc, use convert_pandoc instead

9cd04d7

amueller added 2 commits October 17, 2016 23:51

add asciidoc exporter test

a9f9e4a

add condition on pandoc to test

28b52fb

Add test case

7efb89c

Merge pull request #2 from michaelpacer/change_test_case

cfffb66

Add test case

mpacer closed this Oct 19, 2016

mpacer reopened this Oct 20, 2016

mpacer merged commit f3e79a9 into jupyter:master Oct 20, 2016

asciidoc export #355

asciidoc export #355

Conversation

amueller commented Aug 10, 2016

takluyver Aug 10, 2016

Choose a reason for hiding this comment

amueller Aug 10, 2016 • edited Loading

Choose a reason for hiding this comment

takluyver commented Aug 10, 2016

amueller commented Aug 10, 2016

amueller Aug 10, 2016

Choose a reason for hiding this comment

amueller commented Aug 22, 2016

minrk commented Aug 23, 2016

amueller commented Aug 23, 2016

mpacer commented Aug 25, 2016

amueller commented Aug 25, 2016

takluyver commented Aug 26, 2016

amueller commented Aug 30, 2016

mpacer commented Aug 30, 2016

amueller commented Aug 30, 2016

mpacer commented Aug 30, 2016

mpacer commented Aug 30, 2016

mpacer commented Aug 30, 2016

amueller commented Aug 30, 2016

mpacer commented Aug 31, 2016

mpacer commented Oct 12, 2016 • edited Loading

amueller commented Oct 14, 2016

Carreau commented Oct 17, 2016

mpacer commented Oct 17, 2016

mpacer commented Oct 17, 2016

amueller commented Oct 17, 2016

amueller commented Oct 18, 2016 • edited Loading

mpacer commented Oct 18, 2016

mpacer commented Oct 18, 2016

amueller commented Oct 18, 2016

amueller commented Oct 18, 2016

amueller commented Oct 18, 2016

takluyver commented Oct 18, 2016

mpacer commented Oct 19, 2016

mpacer commented Oct 19, 2016

mpacer commented Oct 19, 2016

amueller commented Oct 19, 2016

amueller Aug 10, 2016 •

edited

Loading

mpacer commented Oct 12, 2016 •

edited

Loading

amueller commented Oct 18, 2016 •

edited

Loading