Skip to content

Fix nbconflux for nbconvert>=6.0#47

Open
nayyarv wants to merge 4 commits intoadtech-labs:masterfrom
nayyarv:varun/fixnbconflux
Open

Fix nbconflux for nbconvert>=6.0#47
nayyarv wants to merge 4 commits intoadtech-labs:masterfrom
nayyarv:varun/fixnbconflux

Conversation

@nayyarv
Copy link
Copy Markdown

@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
Copy Markdown

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
Copy Markdown

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
Copy Markdown

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
Copy Markdown

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
Copy Markdown

@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
Copy Markdown
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
Copy Markdown

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