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 recently occurred type error in AbstractSlideBuilder #174

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

Conversation

lpirl
Copy link
Contributor

@lpirl lpirl commented Aug 25, 2023

Just experienced this

$ sphinx-build -M slides "." "build" -j auto -n -a
…
/…/slides/venv/lib/python3.11/site-packages/hieroglyph/builder.py:126: RemovedInSphinx80Warning: Sphinx 8 will drop support for representing paths as strings. Use "pathlib.Path" or "os.fspath" instead.
  doctree.attributes.get('source')[len(self.srcdir) + 1:].count('/')

Exception occurred:
  File "/…/slides/venv/lib/python3.11/site-packages/hieroglyph/builder.py", line 135, in post_process_images
    if node['uri'].startswith(self.outdir):
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

and think the proposed change is good enough.

Thanks for considering.

Copy link
Contributor

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

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

Recently encountered this in my project, this patch seems to work 👍

@@ -132,7 +132,7 @@ def post_process_images(self, doctree):
node['candidates'] = ('*',)

# fix up images with absolute paths
if node['uri'].startswith(self.outdir):
if node['uri'].startswith(str(self.outdir)):
node['uri'] = '/'.join(
relative_base + [
node['uri'][len(self.outdir) + 1:]
Copy link
Contributor

@oliver-sanders oliver-sanders Aug 29, 2023

Choose a reason for hiding this comment

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

This one can fail too:

Suggested change
node['uri'][len(self.outdir) + 1:]
node['uri'][len(str(self.outdir)) + 1:]

There is a third use of self..outdir in copy_static_files below but that should be fine:

>>> os.path.join('a', Path('b'), 'c')
'a/b/c'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, thanks.

By the way, of course all my PRs are available in https://github.com/lpirl/hieroglyph if that is of any interest to anyone.

in particular: "TypeError: startswith first arg must be str or a tuple
of str, not _StrPath"
@lpirl lpirl force-pushed the fix-builder-node-uri-startswith branch from ef7a6fd to d75c550 Compare August 29, 2023 15:28
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.

2 participants