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

Changed import to enable sphinx-needs 4.1 compatibility #83

Conversation

MaximilianSoerenPollak
Copy link
Contributor

@MaximilianSoerenPollak MaximilianSoerenPollak commented Dec 6, 2024

This is the third 'split' PR that was discussed for splitting the big one into smaller ones. This one here #80

This PR changes the import of make_hashed_id to the now only internal version _make_hashed_id and adapts the arguments to fit as well.
Unsure if this is the best way to do it, or if maybe upstream the function should be exported again.

fixes #78


Note: This PR has a similar issue as #82 issue being that the 8.1.3 Sphinx tests fail, with the same error message.

FAILED tests/test_custom_template.py::test_custom_koi8_template[test_app0] - sphinx.errors.ThemeError: An error happened in rendering the page index.
FAILED tests/test_custom_template.py::test_custom_utf8_template[test_app0] - sphinx.errors.ThemeError: An error happened in rendering the page index.
FAILED tests/test_custom_template.py::test_custom_template[test_app0] - sphinx.errors.ThemeError: An error happened in rendering the page index.

More exact:

E           sphinx.errors.ThemeError: An error happened in rendering the page index.
E           Reason: TemplateNotFound("'about.html' not found in ['/tmp/tmptxv71yyp/custom_tr_template/_templates', '/home/maxi/tmp/testing/sphinx-test-reports/.nox/tests-3-12-sphinx-8-1-3/lib/python3.12/site-packages/sphinx/themes/basic']")

.nox/tests-3-12-sphinx-8-1-3/lib/python3.12/site-packages/sphinx/builders/html/__init__.py:1201: ThemeError

also here let me know if this can be resolved somehow, as I did not find the 'html' it talks about.

I could remove about.html from the conf.py but unsure if that has further consequences.

@@ -6,7 +6,8 @@

from docutils.parsers.rst import Directive
from sphinx.util import logging
from sphinx_needs.api import make_hashed_id
from sphinx_needs.api.need import _make_hashed_id
Copy link
Member

Choose a reason for hiding this comment

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

I guess we need here some dynamic import, depending on the installed Sphinx-Needs versions.
Otherwise the new release will not be compatible with SPhinx-Needs <4.

something like

try:
    from sphinx_needs.api.need import _make_hashed_id
except ImportErro: < Sphinx-Needs 4
    from sphinx_needs.api import make_hashed_id as _make_hashed_id

Copy link
Member

Choose a reason for hiding this comment

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

Added already a fix by commit.

@danwos danwos marked this pull request as ready for review December 16, 2024 07:17
@danwos
Copy link
Member

danwos commented Dec 16, 2024

Ok, I needed to add some stuff to get tests, linter and linkchecks running again.
That's the price when I don't really care for the maintenance of this package :)

Hopefully, find some time to look at your other PRs as well.

@danwos danwos merged commit b44b0b7 into useblocks:master Dec 16, 2024
4 checks passed
@MaximilianSoerenPollak
Copy link
Contributor Author

Ok, I needed to add some stuff to get tests, linter and linkchecks running again. That's the price when I don't really care for the maintenance of this package :)

Hopefully, find some time to look at your other PRs as well.

Let me know if I can be of any assitance.

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.

Compatibility with Sphinx-needs 4+
2 participants