Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

ipynb -> ipynb transformer #3747

Closed
wants to merge 24 commits into
from

Conversation

Projects
None yet
7 participants
Owner

Carreau commented Jul 23, 2013

That's basically split Exporter in 2 class, no more, no less. Exporter now call sometime BaseExpoter.super().same_method()

Exporter Also renamed to TemplateExporter

Owner

ivanov commented Jul 24, 2013

AttributeError: 'module' object has no attribute 'nbconvert' according to Travis

Contributor

wolever commented Jul 24, 2013

Patch to fix this:

diff --git a/IPython/nbconvert/exporters/exporter.py b/IPython/nbconvert/exporters/exporter.py
index 244b6de..8f2035a 100755
--- a/IPython/nbconvert/exporters/exporter.py
+++ b/IPython/nbconvert/exporters/exporter.py
@@ -40,7 +40,7 @@
 from IPython.nbconvert import filters


-from IPython.nbconvert.exporter.baseexporter import BaseExporter, ResourcesDict
+from .baseexporter import BaseExporter, ResourcesDict

 #-----------------------------------------------------------------------------
 # Globals and constants
Contributor

janschulz commented Jul 25, 2013

Could this transformer be used to implement a "remove output before commit"-hook for git? See https://github.com/ipython/nbconvert/pull/142 for the usecase...

Owner

Carreau commented Jul 25, 2013

Yes that the point. Or headlessly rerun all cells.

Le jeudi 25 juillet 2013, JanSchulz a écrit :

Could this transformer be used to implement a "remove output before
commit"-hook for git? See ipython/nbconvert#142https://github.com/ipython/nbconvert/issues/142for the usecase...


Reply to this email directly or view it on GitHubhttps://github.com/ipython/ipython/pull/3747#issuecomment-21545488
.

Owner

Carreau commented Aug 9, 2013

Rebased and cleaned. (actually redone from scratch).
Some config might be changed form Exporter.foo= to BaseExporter.foo= but this is unnecessary.

I'm wondering if we shouldn't have a dummy IPynb2IPynbExporter class that inherit from BaseExporter just in case we want custom logic later.

Ping @jdfreder

Owner

jdfreder commented Aug 10, 2013

Ahh I was waiting for this 😄
I'll take a look right now

Owner

jdfreder commented Aug 10, 2013

It looks good, I have a couple of comments.

  • Right now one can't use the baseexporter from the CLI Do we want to add this functionality now? I personally think this would be great addition (perfect example is the cherry picker plugin I wrote). Maybe an alias like --to notebook. To do so a matching entry would need to be added to exporter.py in the exporter_map.
  • Also, it would be nice if the FilesWriter knew how to write a notebook node object if it recieved it as input. If not maybe a NotebookFilesWriter that inherits from FilesWriter. I prefer the FilesWriter knowing how to write a notebook node object (see small diagram below).

I'm wondering if we shouldn't have a dummy IPynb2IPynbExporter class that inherit from BaseExporter just in case we want custom logic later.

I think I would wait to do that. Most of the logic used in the baseexporter applies to any type of conversion. Since we still need to write some importing stuff, maybe we could do something like this:

(sorry this is so crappy, I drew it in MS Paint)
diagram

Owner

minrk commented Aug 10, 2013

naming question: should the base be Exporter and the current Exporter be TemplateExporter? Seems more precisely descriptive to me.

Owner

jdfreder commented Aug 10, 2013

Exporter and the current Exporter be TemplateExporter? Seems more precisely descriptive to me.

Agreed

Owner

Carreau commented Aug 10, 2013

Right now one can't use the baseexporter from the CLI Do we want to add this functionality now? I personally think this would be great addition (perfect example is the cherry picker plugin I wrote). Maybe an alias like --to notebook. To do so a matching entry would need to be added to exporter.py in the exporter_map.

I think we have 5 month, we better have to wait for BaseExporter to be done before jumping into writing something we will have to do from scratch.

Also, it would be nice if the FilesWriter knew how to write a notebook node object if it recieved it as input. If not maybe a NotebookFilesWriter that inherits from FilesWriter. I prefer the FilesWriter knowing how to write a notebook node object (see small diagram below).

Read and write of .ipynb should use nbformat, which already know how to write .ipynb. I also want NbValidator to be used along the way.

I think I would wait to do that. Most of the logic used in the baseexporter applies to any type of conversion. Since we still need to write some importing stuff, maybe we could do something like this:

Not completely sure about that, I don't quite see what rst2ipynb will reuse in base exporter..

naming question: should the base be Exporter and the current Exporter be TemplateExporter? Seems more precisely descriptive to me.

Ask @ellisonbg he asked @jdfreder to remove the Template suffix.

Owner

jdfreder commented Aug 10, 2013

Ask @ellisonbg he asked @jdfreder to remove the Template suffix.

I think that was only in the context of export to something other than a notebook.

Read and write of .ipynb should use nbformat,

Agreed, I was think of adding a check to FilesWriter to check the input. Something like this maybe:

    from IPython.nbformat import current as nbformat

And in the write method:

            # Write conversion results.
            if isinstance(output, NotebookNode):
                self.log.info("Writing notebook to %s", dest)
                with io.open(dest, 'w', encoding='utf-8') as f:
                    nbformat.write(output, fp=f, format='ipynb')
            else:
                self.log.info("Writing %i bytes to %s", len(output), dest)
                with io.open(dest, 'w', encoding='utf-8') as f:
                    f.write(output)
            return dest

writing something we will have to do from scratch.

I'm confused, I was just suggesting adding an entry to export.py, not doing the all the import stuff now, putting that off for later work.

I don't quite see what rst2ipynb will reuse in base exporter..

I think just from_file and from_filename method signatures (not the code inside the methods).

Owner

Carreau commented Aug 16, 2013

And rebased again.

Owner

jdfreder commented Sep 1, 2013

Other than that, it looks good to me. This should get merged so it doesn't need to be rebased again. @Carreau if you rebase this again, I'll merge it within 24 hrs if there is no objection.

I still think it would be useful to tell the FilesWriter and StdoutWriter how to handle nbnode as input, because without that this is only useful through the API (you can't access the data through the CLI).

Owner

Carreau commented Sep 1, 2013

I'll try, but il I haven't rebased in the next 24h, I might not have internet for a week, so feel free to rebase and open a new PR if needed.

Envoyé de mon iPhone

Le 1 sept. 2013 à 19:11, Jonathan Frederic notifications@github.com a écrit :

Other than that, it looks good to me. This should get merged so it doesn't need to be rebased again. @Carreau if you rebase this again, I'll merge it within 24 hrs if there is no objection.

I still think it would be useful to tell the FilesWriter and StdoutWriter how to handle nbnode as input, because without that this is only useful through the API (you can't access the data through the CLI).


Reply to this email directly or view it on GitHub.

Owner

jdfreder commented Sep 1, 2013

Ok

Owner

jdfreder commented Sep 5, 2013

Ok, it's rebased in #4175 , closing

@jdfreder jdfreder closed this Sep 5, 2013

minrk and others added some commits Sep 9, 2013

@minrk minrk always fire LOCAL_IPS.extend(PUBLIC_IPS)
was in `else`, but should have been `finally` so it aways runs.


Symptom was warnings in parallel.client about `127.0.1.1` not being this machine.
86434a8
@minrk minrk update version-check message in setup.py and IPython.__init__
clearer message that IPython no longer supports 2.6.
1bca05e
@minrk minrk update Python version classifiers:
- drop specific 3.x
- drop 2.6
e128d4e
@minrk minrk update README with dropped 2.6 and 3.2 support
also clarify the checkout steps in development install
e9a19b0
@takluyver takluyver Use Instance trait for user_ns instead of Dict.
Instance(dict) is None by default, unlike Dict().
f954561
@takluyver takluyver Add a test for kernel started without user_ns kwarg.
user_module should not be an instance of DummyMod
4d6e0e5
@ivanov ivanov Merge pull request #4189 from minrk/localinterfaces-finally
always fire LOCAL_IPS.extend(PUBLIC_IPS)
3ce4b25
@minrk minrk Merge pull request #4188 from takluyver/dict-traitlet-none
Allow user_ns trait to be None

The Dict trait converts None to an empty dict, which caused the application to pass in a user_ns to the shell, which then behaves slightly differently, creating a DummyMod instance.
87d41d7
@minrk minrk 1.1 backport stats 7bdb1ae
@minrk minrk don't upload to GitHub in release script
GitHub doesn't have an API for it at the moment
3975d00
@minrk minrk Merge pull request #4190 from minrk/byebye2.6
update version-check message in setup.py and IPython.__init__

clearer message that IPython no longer supports 2.6.
08e02c6
@takluyver takluyver Merge pull request #4186 from mmckerns/master
moved DummyMod to proper namespace to enable dill pickling
a96f9c4
@Carreau Carreau remove executable flag on soem files ccb3fe5
@Carreau Carreau Split exporter in to for ipynb->ipynb 4f0f98f
@Carreau Carreau some trailing whitespace 96e8f30
@Carreau Carreau import baseexporter in init 8106096
@Carreau Carreau Exporter -> TemplateExporter / BaseExporter f52dfb6
@Carreau Carreau fix 1 test 87c3fc1
@jdfreder @Carreau jdfreder + Carreau renamed: exporter.py -> template_exporter.py bb2dc10
@jdfreder @Carreau jdfreder + Carreau renamed: baseexporter.py -> exporter.py 962d234
@jdfreder @Carreau jdfreder + Carreau renamed: template_exporter.py -> templateexporter.py 1c76bd3
@jdfreder @Carreau jdfreder + Carreau Rebase changes made by hand 14cf5fe
@jdfreder @Carreau jdfreder + Carreau Check if class is an instance of Exporter, not TemplateExporter 310bbbe
@jdfreder @Carreau jdfreder + Carreau Fixed Travis, missing import in export.py 9eaff2f
Owner

Carreau commented Sep 12, 2013

#4175 was not merging cleanly anymore. This is just a rebase of it.

@Carreau Carreau reopened this Sep 12, 2013

Owner

Carreau commented Sep 12, 2013

Damed github see the wrong commits.

Owner

jdfreder commented Sep 12, 2013

Another rebase ? 😛 We should merge this soon, I'm tired of seeing this get rebased.

Owner

jdfreder commented Sep 12, 2013

I was able to rebase #4175 without github having trouble, should I close this one again?

@Carreau Carreau closed this Sep 12, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment