search

vidartf / nbsphinx-link

A sphinx extension for including notebook files from outside sphinx source root.
BSD 3-Clause "New" or "Revised" License
37 stars 15 forks source link

Fix of breaking change with `docutils`: `docutils.nodes.reprunicode` removed with version 0.21 #21

Closed fkuehlein closed 6 months ago

fkuehlein commented 6 months ago

Hi,

I like to use nbsphinx-link for building my docs, but got an AttributeError: module 'docutils.nodes' has no attribute 'reprunicode' today. I realized that nbsphinx-link/__init__.py::LinkedNotebookParser uses docutils.nodes.reprunicode, which was removed as of docutils v0.21, as being a 'Python 2 compatibility hack'.

Indeed, just leaving it out will fix the bug, which is what I did in the below commit. Hope to make a useful contribution.

Cheers, F

Edit: See #22 for a better approach maintaining Python 2 compatibility.

antoinecollet5 commented 6 months ago

@fkuehlein I did not see your PR and I opened an issue yesterday #22

Do you think nbspinx_link supports python2 ? Or could we simply delete the problemtic rows as you did ?

Regards Antoine

fkuehlein commented 6 months ago

@antoinecollet5 good point, according to the setup.py Python 2.7 is actually supported. Therefore the option you proposed in #22 is the safe way to go for sure!

Would you like to just make a PR yourself? Then I can close this one.

Best, F

antoinecollet5 commented 6 months ago

Ok, in that case, simply update this PR and link the issue in the PR description. Antoine

antoinecollet5 commented 6 months ago

Why did you close it ?

fkuehlein commented 6 months ago

because I think yours is the better solution and you would want to make a new PR. Could still reopen this and leave it to the repo owner to decide, maybe that makes sense indeed.

antoinecollet5 commented 6 months ago

Well in the mean time I did another PR since you closed this one ^^^ahah

Indeed, what I proposed is probably the way to go. I was suggesting you to modify this PR instead of doing a new. Anyway, I did a new so you can close it again to avoid doing the job twice.

Antoine

fkuehlein commented 6 months ago

haha yea I saw that, but only now understood what you were suggesting. anyway, I'll close it now ...:D