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

Making ipynb support truly agnostics in agreement with Jupyter raise... #1774

Closed
wants to merge 12 commits into from

Conversation

damianavila
Copy link
Member

WIP, but essentially let you use Jupyter (the IPython evolution) generating an ipynb with configurable language agnostic kernel and also populating the notebook metadata in a nikola namespace, so we now have support for onefile and twofiles (splitted meta). In this way, since the read_metadata is implemented, then we don't have to populate the metadata manually (or with jsextension)... all is more easy now just from the command line ;-)
I have to test some things because it is too late now... I will probably complete it tomorrow, but should be enough completed for first review...

@damianavila
Copy link
Member Author

Note to myself... I have to make dicts for each kernel so they have the correct name, language and display_name in the kernelspec...

@@ -213,6 +213,14 @@ class CommandNewPost(Command):
'bbcode, html, textile, txt2tags',
},
{
'name': 'kernel',
Copy link
Member

Choose a reason for hiding this comment

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

Do we have to add a new option that will not apply to most compilers?

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... we need to specify the kernel to build the correct notebook template with the correct kernel specification... if not... the notebook is no validated ok and will fail...
I am open to ideas about how to deal with this that, as you said, is not available for other compilers...
One option would be probably make it only available with ipynb (warn and exit if you try to use it with other compilers... but maybe they are other options...

Copy link
Member

Choose a reason for hiding this comment

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

It should totally fail when used with other compilers. And it should say in the help that it's only important for jupyter, something like

'help': 'Kernel to use for jupyter posts, irrelevant for all other formats' or somesuch

Copy link
Member

Choose a reason for hiding this comment

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

My idea would be to make it -f ipynb@python — this will also be extensible in the future when we have other compilers with multiple formats (or -f pandoc@markdown, for example)

If we do add this parameter, it must also be added in new_page.py.

Copy link
Member Author

Choose a reason for hiding this comment

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

My idea would be to make it -f ipynb@python

This is interesting... I will explore this possibility... we can probably provide the more popular formats: ipynb@python2, ipynb@python3, ipynb@julia, ipynb@r and leave the others kernels as a plugable thing (I mean not in core... there are more than 30 kernel... we can not support all of them in core).

Copy link
Member

Choose a reason for hiding this comment

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

How much work is needed to support a kernel, exactly?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just an ipynb with a different kernel spec... the thing about supporting more than the 4 main ones is that we need to explore how each of the other ~30 kernels specify their kernelspec and be awared of any change in the future...

@damianavila
Copy link
Member Author

@ralsina @Kwpolska this is still WIP, but I am stuck in something and it is late... probably I am missing something obvious but I can not figured out.
First, you need to modify your conf.py and add this line to the COMPILERS:

    "ipynb_python2": ('.ipynb',),

The issue is that when I run nikola new_post -f ipynb_python2 I don't get the modifications I want to introduce overriding the set_kernel_metadata method here: https://github.com/getnikola/nikola/pull/1774/files#diff-2f082ac5337057b98fc6c75abb9c4bb4R84

It just takes the parent method... not adding anything... ideas welcomed.

After that I just need to add the pyhon3, julia and R kernel metadata and it will be ready for review again...

@@ -0,0 +1,10 @@
[Core]
Name = ipynb_python2
Module = ipynb/ipynb_python2
Copy link
Member

Choose a reason for hiding this comment

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

That’s probably not the correct way to write that.

@Kwpolska
Copy link
Member

You must probably return nb_object and use that instance instead.

I would personally prefer a sleeker implementation that would set ipynb_kernelspec and ipynb_language_info class variables which would be read by create_post and set on nb. (I would also try to get rid of classes by using some sort of “metacompilers” that don’t have real modules, BTW)

@damianavila
Copy link
Member Author

You must probably return nb_object and use that instance instead.

OK, I will try it...

I would personally prefer a sleeker implementation that would set ipynb_kernelspec and ipynb_language_info class variables which would be read by create_post and set on nb.

Sound interesting...

(I would also try to get rid of classes by using some sort of “metacompilers” that don’t have real modules, BTW)

Can you elaborate you idea here a little bit more?

@Kwpolska
Copy link
Member

  1. COMPILERS = {'ipynb@python3': ('.ipynb',)}
  2. split on @ and pass python3 as a special parameter to one big ipynb module
  3. store data for each language in dicts inside the plugin
  4. ???
  5. profit

@damianavila
Copy link
Member Author

  1. split on @ and pass python3 as a special parameter to one big ipynb module

Passing the parameter as part of the metadata here: https://github.com/getnikola/nikola/blob/master/nikola/plugins/command/new_post.py#L382?
because if I create an special parameter, I guess I should update the other compilers... then I can drop that parameter from the metadata inside create post...

  1. store data for each language in dicts inside the plugin

OK

@Kwpolska
Copy link
Member

Your idea for 2. makes sense, but we should do it if and only if the compiler has an @ sign in it. That way, we won’t need to edit any other compilers.

@damianavila
Copy link
Member Author

You must probably return nb_object and use that instance instead.

Did not work... but forget about that... I just following the last plan.
Pretty close to be done. Will push soon...

…metadata according to the kernel name (ipynb.py). Also understand the compiler@subcompiler notation and sent the kernel name using the metadata (new_post.py).
@damianavila
Copy link
Member Author

@ralsina @Kwpolska AFAIK, this is working OK, please review...
I probably have to add some docs and raise the version number of the plugin... and probably change the changelog... beside those things... am I missing something else? Thanks!!

@Kwpolska
Copy link
Member

Build failure due to setup.py still including ipynb as a package. Please fix that.

(Also, we should switch to an automated package finder)

@damianavila
Copy link
Member Author

Opps, I forgot that one... I will fix it now...

@Kwpolska
Copy link
Member

Just set it to setuptools.find_packages() and it should work (adjust the
from import?)

Chris Warrick https://chriswarrick.com
Sent from my Galaxy S3.
On 17 Jun 2015 22:03, "Damián Avila" notifications@github.com wrote:

Opps, I forgot that one... I will fix it now...


Reply to this email directly or view it on GitHub
#1774 (comment).

@damianavila
Copy link
Member Author

Just set it to setuptools.find_packages() and it should work (adjust the
from import?)

let's try it...

@damianavila
Copy link
Member Author

There is some error in appveyor... but I don't know if it is related with the find_packages() thing... on linux tests passed... any hints?

@Kwpolska
Copy link
Member

It looks like we made it install our test suite in site-packages… I’ll fix
this later today (or you can try yourself); we need to limit the package
finder to the nikola package.

Chris Warrick https://chriswarrick.com
Sent from my Galaxy S3.
On 17 Jun 2015 23:48, "Damián Avila" notifications@github.com wrote:

There is some error in appveyor... but I don't know if it is related with
the find_packages() thing... on linux tests passed... any hints?


Reply to this email directly or view it on GitHub
#1774 (comment).

'nikola.plugins.task.sitemap',
'nikola.plugins.template',
],
packages=find_packages(),
Copy link
Member

Choose a reason for hiding this comment

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

      packages=find_packages(exclude=('tests',)),

Also, in the future, please work on a branch in getnikola/nikola (not your own fork) so we can collaborate.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, in the future, please work on a branch in getnikola/nikola (not your own fork) so we can collaborate.

Yes, I will take into account next time... make easier to collaborate, I agree...

Copy link
Member Author

Choose a reason for hiding this comment

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

In fact, closing this one and opening a new PR in the getnikola/nikola becuase probably there are other things to fix...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants