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

Use absolute path to set .ipynb file as dependency #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mgeier
Copy link

@mgeier mgeier commented Feb 16, 2021

Getting the relative path seems to be broken, but Sphinx seems to be fine with absolute paths as well.

With a bit of luck, this fixes mgeier/sphinx-last-updated-by-git#26.

@mgeier
Copy link
Author

mgeier commented Feb 16, 2021

I'm actually not sure whether absolute paths are "officially supported" by Sphinx.

Maybe it would be better to fix the relative paths?

Feel free to close this PR and replace it with one that fixes the relative paths.

@vidartf
Copy link
Owner

vidartf commented Feb 19, 2021

I'm not sure if I follow what the bug is. Would you mind giving a bit more detail?

@mgeier
Copy link
Author

mgeier commented Feb 19, 2021

Sure. I didn't look too closely into this anyway, so it might be good to take a closer look:

When adding this to the nbsphinx-link source code:

def _env_updated(app, env):
    for docname in env.found_docs:
        print('dependencies for', docname)
        for dep in env.dependencies[docname]:
            print('   ', dep)

... and then adding the following to the setup() function:

    app.connect('env-updated', _env_updated)

... and running Sphinx on the nbsphinx-link docs, I get this output:

...
dependencies for introduction
    ../../notebooks/introduction.ipynb
    /home/[...]/nbsphinx-link/notebooks/smile.png
    ../../notebooks/images/smile.png
    /home/[...]/nbsphinx-link/notebooks/images/smile.png
    notebooks/introduction.ipynb
    /home/[...]/nbsphinx-link/notebooks/images/subsubdir/smile.png
    smile.png
    ../../notebooks/images/subsubdir/smile.png
    images/smile.png
    ../../notebooks/smile.png
    images/subsubdir/smile.png
...

The problem is the path notebooks/introduction.ipynb, which doesn't exist (relative to the source directory).

However, what I didn't know before, there seems to already be a correct path for the notebook: ../../notebooks/introduction.ipynb.

I don't know why there are two different paths for the same notebook, but I know that one of them is wrong, which I think is the cause for the issue I got: mgeier/sphinx-last-updated-by-git#26.

Some of the other dependencies seem to be duplicated as well, but that's probably a separate issue.

@mgeier
Copy link
Author

mgeier commented Apr 21, 2021

Any news on this?

@mgeier
Copy link
Author

mgeier commented Jun 1, 2021

Ping?

@vidartf
Copy link
Owner

vidartf commented Jun 4, 2021

I can really see that this will change anything: Currently the code uses path, which was set using

path = utils.relative_path(None, abs_path)

When None is the first argument, this function only normalizes the path to the format that docutils like. It should still be an absolute path...

@mgeier
Copy link
Author

mgeier commented Jun 16, 2021

It should still be an absolute path...

I don't know about that, but my example clearly shows that there are (invalid) relative paths. Can you reproduce this?

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