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

Download embedded HTML from the Menu #1039

Merged
merged 16 commits into from
Aug 12, 2017

Conversation

gabyx
Copy link
Contributor

@gabyx gabyx commented Aug 8, 2017

This is a very handy export functionality, which lets the user export an embedded html by the
already included htmlembed.py exporter.

This only works in version 5.1, since the milestone for this merged (into master) pull request is 5.1
jupyter/notebook#2706

Problems:
The only problem which the extension has is that relative paths in markdown images can only be resolved when jupyter is started in the same folder as the notebook resides.

To overcome this:
I wanted to pass the notebook file path EmbedImages/Test.ipynb from the url http://localhost:8888/nbconvert/html_embed/EmbedImages/Test.ipynb?download=true to the exporter (resources[metadata]) such that I can adapt the relative paths when the exporter runs and opens the relative paths in the markdown links (e.g. ![](graphics/pic.png))

but that needs change in the notebook
notebook/export/handlers.py: .

class NbconvertFileHandler(IPythonHandler):

    SUPPORTED_METHODS = ('GET',)
    
    @web.authenticated
    def get(self, format, path):
    ...
            output, resources = exporter.from_notebook_node(
                model['content'],
                resources={
                    "metadata": {
                        "name": name[:name.rfind('.')],
                        "modified_date": (model['last_modified']
                            .strftime(text.date_format)),
                        "path" : path   ### < Handing over the path here...
                    },
                    "config_dir": self.application.settings['config_dir'],
                }
            )

Can that be achieved differently?

@jcb91
Copy link
Member

jcb91 commented Aug 9, 2017

Thanks for this PR, looks like a neat idea!

Can that be achieved differently?

As far as I can see (I don't have a huge amount of experience with nbconvert), you are correct in the sense that the call to from_notebook_node currently passes no path information, so a modification to either the handler or the contents manager (which provides the model, which could add an os_path field) would be necessary. I can't see massive objections to adding that resource in the notebook calls to make it available for exporters in general, as it seems like it could be a useful thing to have more generally. However, this could equally be achieved as a server extension as opposed to needing to modify the notebook source itself. The serverextension could either replace the existing handler(s) with ones which add the relevant resource:

def load_jupyter_server_extension(nbapp):
    for host_rgx, urlspecs in nbapp.web_app.handlers:
        for urlspec in urlspecs:
            if urlspec.handler_class is NbconvertPostHandler:
                urlspec.handler_class = NbconvertPostHandlerWithPath
            elif urlspec.handler_class is NbconvertFileHandler:
                urlspec.handler_class = NbconvertFileHandlerWithPath

or monkey-patch the nbapp.contents_manager._notebook_model method to get it to add an extra field:

def load_jupyter_server_extension(nbapp):
    original_notebook_model = nbapp.contents_manager._notebook_model
    def _notebook_model_with_path(self, path, content=True):
        model = original_notebook_model(self, path, content=True)
        model['os_path'] = self._get_os_path(path)
        return model
    nbapp.contents_manager._notebook_model = _notebook_model_with_path

@@ -0,0 +1,7 @@
Type: IPython Notebook Extension
Copy link
Member

Choose a reason for hiding this comment

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

can we have Jupyter Notebook Extension, for future-proofing?

README.md Outdated
@@ -1,723 +0,0 @@
Jupyter notebook extensions
Copy link
Member

Choose a reason for hiding this comment

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

You seem to have mistakenly removed the main README.md - could we get this reinstated?

@@ -0,0 +1,7 @@
Export Embedded HTML
Copy link
Member

Choose a reason for hiding this comment

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

Could we alter this to something like "Export HTML with images embedded" to clarify what's going on?

@gabyx
Copy link
Contributor Author

gabyx commented Aug 10, 2017

All your comments are very good, I will make the suggested changes!
I will get e glimpse on the server extentions :-), sounds feasible :-)

Maybe it would be better to attach a markdownimage->base64 preprocessor inside html_embed.py

@gabyx
Copy link
Contributor Author

gabyx commented Aug 10, 2017

Maybe we can already merge this extension even though it is not perfect. Maybe in a later step we can improve it, by making it agnostic to the path where the working directory of jupyter is ...
Is there a way of getting the current working directory of jupyter. Inside the extension?
I am not familiar with the whole architecture of jupyter server and so on, but does it seem to be odd to use open(path) inside the extension to open a file? Does that work if jupyter server is run somewhere else? I mean , is it meaningful to operate on files inside the converter?

@jcb91
Copy link
Member

jcb91 commented Aug 11, 2017

Is there a way of getting the current working directory of jupyter. Inside the extension?

No, I don't think there's any way to get it in javascript. In a python serverextension, sure, since it's just the python working directory, but that's not terribly useful, I guess, without knowing the directory of the notebook also, which the client-side js isn't passing.

I am not familiar with the whole architecture of jupyter server and so on, but does it seem to be odd to use open(path) inside the extension to open a file? Does that work if jupyter server is run somewhere else?

This is a bit of an issue, yes. In general, the Jupyter server operates on a kind of abstracted file system provided by a FileManager class. The default is class provides access to a regular file tree, but there are (I think?) also managers which provide access to, for example, cloud services like dropbox or google drive. For those file managers, the os path becomes not really defined, and instead we're talking about some kind of URI which we'd presumably need to use to locate relatively-linked files, with no relation to the server's current working directory.

I mean , is it meaningful to operate on files inside the converter?

I guess this comes down to how nbconvert expects to operate, but I suspect that including relatively-linked images is a pretty common operation - for example, I think this may be done in requesting a zipped download. In such cases, nbconvert must require a way to resolve relative links, but I suspect you might find better support asking at nbconvert than my half-guess answers here 😉

@jcb91
Copy link
Member

jcb91 commented Aug 11, 2017

Maybe we can already merge this extension even though it is not perfect

Sure, we can always iterate on this later, it already provided some useful functionality.

…oading)

put it to 5.x !

s Please enter the commit message for your changes. Lines starting
@gabyx
Copy link
Contributor Author

gabyx commented Aug 12, 2017

I corrected the version to 5.x because the loading makes problem, with 5.x everything works.
I think in this stat, it should be mergable :-)

@juhasch
Copy link
Member

juhasch commented Aug 12, 2017

Could you add a check for the notebook version being at least 5.1 ?
I guess Jupyter.version >= "5.1.0" would do it.

For the image path issue: This probably should be handled in the exporter. I'll take a look.

Otherwise 👍

@gabyx
Copy link
Contributor Author

gabyx commented Aug 12, 2017

where should that check go? in the embeddedhtml.py converter?
or in java script?

@juhasch
Copy link
Member

juhasch commented Aug 12, 2017

PR #1044 should do it for the exporter. This is in Python, it should know about the correct path.

@gabyx
Copy link
Contributor Author

gabyx commented Aug 12, 2017

var v = Jupyter.version.split(".")
        if(Number(v[0])*10+ Number(v[1])  < 51)
        {
          # alert???
          return
        }

is there some funtionaily in Jupyter to alert the user some how about the error?

@juhasch
Copy link
Member

juhasch commented Aug 12, 2017

I would check for the version, if not at least 5.1.0 do a console.log('Notebook version 5.1.0 or higher required for this extension) and not install the menu entry at all.

@juhasch
Copy link
Member

juhasch commented Aug 12, 2017

Fair enough. Let's merge once the checks have completed.

@juhasch juhasch merged commit dd47f82 into ipython-contrib:master Aug 12, 2017
@gabyx gabyx deleted the download_embedded branch August 12, 2017 16:54
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.

3 participants