Thanks for this PR!

The amount of imports seems a bit excessive to me, but if you think that's the right thing to do, I'll take it.

@jenshnielsen

The amount of imports seems a bit excessive to me, but if you think that's the right thing to do, I'll take it.

We could limit the fix to just the missing imports from sphinx.util but then you risk that some of the other modules make similar changes in the future. I guess that is a matter of preference, but I favour explicit imports over that risk.

Not really sure why CI is failing and if there is anything I can to to fix it?

I don't think this change harms anything, but I also can't find any documentation that so-called "implicit" imports are a bad practice in Python. I would love to get the immediate issue fixed though, as it's blocking my builds (and based on the number of cross-references to #825, a lot of other people's too).

@zmbc If you run pyright on the code base you will find that every single one of these (except that imports from sphinx.utils) are higlighted as missing explicit imports.
Imagine that you have modules

a\__init__.py
a\b\__init__.py

With class C defined in a\b\__init__.py

Now if you do:

import a
a.b.C()

You are referencing the module b that you have not imported.
That only works because the a\__init_.py file imports b and that import is cached. As far as your code is concerned there is no explicit import of the module that you use and the import is implicit.

Given this situation you can:

1. Add all imports explicitly for all modules that you use
2. Hope and pray that none of your upstream dependencies or other parts of your codebase changes its imports to stop importing things that you rely on.

You are free to do as you want but imho the second option is just a recipe to introduce latent bugs in your code to save a few lines of typing.

Interesting! Based on what I've just read about the semantics of importing a module in Python, I see why pyright does this. That said, it can easily lead to needing a lot of explicit exports, since there is apparently no way to tell Python to import all submodules recursively.

I'm also curious, but haven't checked, how other type-checkers compare in this area. I get the sense that pyright is exceptionally strict (e.g. from this page) and its documentation about this feature is relatively minimal.

A lot of explicit imports are only needed if a module is using a lot of external API's (which maybe is a sign of some other design problem? module too big? other library not consolidating its public API enough?). I can't recall seeing importing a module and relying its subimports in other Python projects, though I think that pattern is fair to use as long as the upstream project documents it as supported. Note though that in this case a Sphinx maintainer has already described this case as an "error with nbsphinx", implying that Sphinx does not expect downstream projects to rely on these subimports. (Edit: the Sphinx maintainer is more explicit in this comment).

What are the next steps to get this merged? It appears there are unrelated CI failures, do those need to be resolved first?

@mgeier @zmbc I am not sure what causes the current test failures and I don't really have the time to investigate it at the moment

In one case at least the failure seems to be due to another Sphinx extension's incompatibility with Sphinx 8.2. In particular, Sphinx is generating a deprecation warning as described in felix-hilden/sphinx-codeautolink#173. The nbsphinx CI sets deprecation warnings to errors for some of the jobs. Maybe an exception could be added for that case since it is not nbsphinx's issue to fix.

Other jobs have other errors like:

/home/runner/work/nbsphinx/nbsphinx/doc/code-cells.ipynb: WARNING: Could not match transformation of `sys` on source lines 1-1, source:
import sys

that also seem to come from sphinx-codeautolink but I don't see an issue like that on sphinx-codeautolink's tracker and I am not sure what is going on there.

Fixed some of the warnings:

• Suppressed external deprecation warning as suggested above
• Changed jobs that run with sphinx 5 to explicitly run with python 3.12. The default for python 3 on github actions seems to have changed to 3.13 and Sphinx 5 uses imghrd module that was removed in 3.13
• Changed the jobs that are ment to test the current Sphinx version to be 3.11 - 3.13 and added an explicit 8.1 job since Spinx 8.2 only supports 3.11 and up.
• Add read the docs config now required https://about.readthedocs.com/blog/2024/12/deprecate-config-files-without-sphinx-or-mkdocs-config/

We are now hitting missinglinkelectronics/sphinxcontrib-svg2pdfconverter#26

And it seems the only options for that is to wait for a fix to sphinxcontrib-svg2pdfconverter or remove the usage of that package.

See jenshnielsen#1 for tests that run without waiting for approval

With a temporary fork of sphinxcontrib-svg2pdfconverter CI is now passing except a link check that looks like a real network issue that will hopefully be resolved

Nice, so I think there is no rush to merge this since in practice nbsphinx can't be used with Sphinx 8.2 without the sphinxcontrib-svg2pdfconverter fix being merged and released any way. Maybe a separate PR could pin sphinx<8.2 so the CI can pass for other PRs in the meantime, but the sphinxcontrib-svg2pdfconverter fix looks pretty straightforward so hopefully it will be available soon.

Thanks @jenshnielsen for the fixes and all others for the suggestions!

It really has been quite annoying to catch up with Sphinx breakages over the last few minor releases ... I hope this will get better at some point.

I'm open for suggestions how to improve this from our side in the future.

Maybe a separate PR could pin sphinx<8.2 [...]

I think pinning upper bounds is in general a bad idea (it is better for downstream projects to do the pinning, which many already did for Sphinx < 8.2, as visible in #825), but in this very case it might be the best (temporary) solution. I've created #828 for this.

I would like to make an "unbroken" release soon, fixing the Sphinx 8.2 incompatibilities can then happen at a slower pace.

I have created a new nbsphinx release which now gives us ample time to work on the remaining compatibility issues.

@jenshnielsen Can you please reduce this PR back to its original change of making implicit imports explicit?

Can you please make separate PRs for the unrelated changes (if that makes sense)?

I'm not sure about how to handle warnings in CI in the future, I have the feeling that we should re-think the current approach.

@jenshnielsen Can you please reduce this PR back to its original change of making implicit imports explicit?
Done, This now only contains the import changes

Can you please make separate PRs for the unrelated changes (if that makes sense)?
I am not sure that there is anything worth keeping. The only other changes not on main are

1. Use my fork of sphinxcontrib-svg2pdfconverter which is not something that we want to merge anyway.
2. Silence warnings from sphinx-codeautolink. If you don't want to support sphinx 8.2. before this is fixed not much point in merging this

I'm not sure about how to handle warnings in CI in the future, I have the feeling that we should re-think the current approach.

To me it seem like things are harder than they should be mostly because the CI of nbsphinx actually includes warnings from third party sphinx plugins that are not requirements of nbsphinx. Would it be an idea to create a more minimal project sphinx project to use in CI that does not depend on any more plugins

the CI of nbsphinx actually includes warnings from third party sphinx plugins that are not requirements of nbsphinx

It's true that they are not strict requirements in a package management sense, but I do care that they keep working well together with nbsphinx. I do consider it a bug if any part of the nbsphinx docs doesn't work as expected, even if the underlying cause is in one of those 3rd party packages.

I know that warnings might not compromise the resulting docs, but they might become errors at a later point, and I think it's better to become aware of them sooner rather than later. And also, I would like to enable nbsphinx users to enable warnings on their projects. I think the more people take warnings seriously, the better.

Having said that, I'm sure there are ways how the CI can be improved, and I'm open for PRs!