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

Fix nbconflux for nbconvert>=6.0 #47

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

nayyarv
Copy link

@nayyarv nayyarv commented Oct 15, 2021

Hi, this is related to #45 and #46

I based this off the PR there but ended up fixing the actual issue since I believe the fix didn't quite work as expected since traitlets act a bit oddly if you're not used to them.

I took @makammoun's changes and a fixed it to actually work against a confluence I have access to. I provided the integration test I used, but I couldn't work out how to actually have this work for anyone. It might not be a bad idea to have a nbconflux confluence for vericast so you can run the test in CI.

I have removed the style includes from the confluence templates as this has been deprecated for years now (no more custom style sheets). There were a few minor cleanups too.

You'll probably want to do a new release as 0.7.0 is likely mostly broken for people.

@eric-tramel
Copy link

Can confirm, that of the existing forks etc, this is the PR that truly works. Verified the operation of this using API Token keys. There are two limitations, however:

  1. MathJaX does not render (Atlassian Cloud as of April 4, 2022 update).
  2. Cell syntax highlighting is not set properly. Code cells are rendered as raw text (however, one can a posteriori come back and set the appropriate language, but this seems burdensome for large notebooks).

Would be a great start to getting around the stale nbconvert problems. However, really need maintainers to come back and give some feedback and/or take one of these PRs.

@chblanc
Copy link

chblanc commented May 19, 2022

Can confirm that this fork isn't working properly with nb_convert=6.5.0. The notebook_to_page command fails with the dreaded TraitError: The 'exporter' trait of a ConfluencePreprocessor instance expected a ConfluenceExporter, not the NoneType None that other users have been experiencing.

@ajstewart
Copy link

Can confirm that this fork isn't working properly with nb_convert=6.5.0. The notebook_to_page command fails with the dreaded TraitError: The 'exporter' trait of a ConfluencePreprocessor instance expected a ConfluenceExporter, not the NoneType None that other users have been experiencing.

@chblanc and for anyone else reading this, this branch worked for me today with nbconvert==6.5.0 (still with Eric's caveats above). My only issue was downgrading bleach<5.0 as 5.0 changed the way CSS styles are handled and will break nbconflux.

@chblanc
Copy link

chblanc commented May 25, 2022

@ajstewart I stand corrected. It turns out I accidentally installed the wrong branch. I can now confirm that this is working with bleach=4.1.0.

@kristofer-hannesson
Copy link

@chblanc or @nayyarv can you please tell me how to correctly install the branch? I've been trying variations of commands but can't get it to work correctly.

@nayyarv
Copy link
Author

nayyarv commented Jul 19, 2022

I just checked out the branch from my repo and used it directly from there. You shouldn't need to install anything.

@kristofer-hannesson
Copy link

kristofer-hannesson commented Jul 20, 2022

For anyone who wants a quick way to install the branch from this fork this command worked for me

pip install -e "git+https://github.com/nayyarv/nbconflux.git@varun/fixnbconflux#egg=nbconflux"

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.

5 participants