Installation requires preinstalled Sphinx

[danwos]

I'm unable to install breathe with a fresh new virtual environment from locally checked out git repo .

Error message after pip install breathe ,:

    ERROR: Command errored out with exit status 1:
     command: /home/daniel/workspace/sphinx/breathe/.venv/bin/python -c 'import io, os, sys, setuptools, tokenize; sys.argv[0] = '"'"'/tmp/pip-req-build-mqj3ctdq/setup.py'"'"'; __file__='"'"'/tmp/pip-req-build-mqj3ctdq/setup.py'"'"';f = getattr(tokenize, '"'"'open'"'"', open)(__file__) if os.path.exists(__file__) else io.StringIO('"'"'from setuptools import setup; setup()'"'"');code = f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' egg_info --egg-base /tmp/pip-pip-egg-info-cjlbra1y
         cwd: /tmp/pip-req-build-mqj3ctdq/
    Complete output (15 lines):
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-req-build-mqj3ctdq/setup.py", line 10, in <module>
        from breathe import __version__
      File "/tmp/pip-req-build-mqj3ctdq/breathe/__init__.py", line 1, in <module>
        from . import directives
      File "/tmp/pip-req-build-mqj3ctdq/breathe/directives.py", line 1, in <module>
        from breathe.directive.base import create_warning
      File "/tmp/pip-req-build-mqj3ctdq/breathe/directive/base.py", line 1, in <module>
        from breathe.finder.factory import FinderFactory
      File "/tmp/pip-req-build-mqj3ctdq/breathe/finder/__init__.py", line 1, in <module>
        from breathe.project import ProjectInfo
      File "/tmp/pip-req-build-mqj3ctdq/breathe/project.py", line 4, in <module>
        from sphinx.application import Sphinx
    ModuleNotFoundError: No module named 'sphinx'

Reason is this line in setup.py:
from breathe import __version__

breathe/__init__.py, which contains the __version__ attribute, also contains this line:
from sphinx.application import Sphinx.

So it looks like this line forces pip/setuptools to import already sphinx, which may not be installed yet.

Workarounds

1. Remove from setup.py from breathe import __version__ and set a fix value for version.
2. Install sphinx in advance

Solutions idea

Maybe put __version__ into a separate version.py and let __init__.py import it.
setup.py could then import directly from version.py.

[vermeeren]

@danwos I like your proposal of a version.py file, that seems to avoid duplication and fix the build-time dependency on Sphinx. (There may still be deps elsewhere on Sphinx, I do not know.)

There is however a bunch of other stuff with higher priority, so this'll be a "patches welcome" for now.

Thanks

[2bndy5]

I found a way to automate the version number completely. Using setuptools_scm at build time will automatically store the tag from the commit in the package metadata. According to PEP 440, the preferred way to fetch a package version is using

from importlib.metadata import version
__version__ = version("breathe")

Some tweaks are needed to the above snippet if installing with python v3.7 or older. Also, if installed from a non-tagged commit, the version number used will be much like what git describe outputs,
<latest-tag>+<commits-since>-dev<increment>.<abbreviated-sha>.

This form can still be used by the docs' conf.py to signal weather or not the docs are of stable release or a dev build.

I will submit a PR

[2bndy5]

The version.py proposal is implemented in #766 via the setuptools_scm pkg at build time.

[svenevs]

Pending discussion in #766 really this is quite a minor hangup but should be resolved to support installing breathe from github urls. --no-build-isolation flag for pip cannot always save you (e.g., if I have a breathe git url in my install_requires I cannot install).

Option 1: Dirty Hack

What I do is just defer imports. You also don't need the type hint in breathe/__init__.py -- it's only marginally useful, but shouldn't be a reason to require sphinx for install.

diff --git a/breathe/__init__.py b/breathe/__init__.py
--- a/breathe/__init__.py
+++ b/breathe/__init__.py
@@ -1,13 +1,13 @@
-from breathe.directives.setup import setup as directive_setup
-from breathe.file_state_cache import setup as file_state_cache_setup
-from breathe.renderer.sphinxrenderer import setup as renderer_setup
-
-from sphinx.application import Sphinx
-
 __version__ = "4.31.0"
 
 
-def setup(app: Sphinx):
+def setup(app: "sphinx.application.Sphinx"):
+    # Defer breathe imports so that install tools needing breathe.__version__
+    # do not need sphinx installed.
+    from breathe.directives.setup import setup as directive_setup
+    from breathe.file_state_cache import setup as file_state_cache_setup
+    from breathe.renderer.sphinxrenderer import setup as renderer_setup
+
     directive_setup(app)
     file_state_cache_setup(app)
     renderer_setup(app)

Problem solved. Possibly some # noqa comments for linting checks, marginal loss of mypy coverage. But, well, we all know it's sphinx.application.Sphinx...

Option 2: some sort of version.py

You could go down this route and create a breathe/breathe_version.py with BREATHE_VERSION = "X.Y.Z" in it. This would enable you to, in __init__.py, do from breathe.breathe_version import BREATHE_VERSION and set __version__ = BREATHE_VERSION.

However, on the install side that means you'll need to modify sys.path so that you can only import breathe_version.py and not do a from breathe.breathe_version import BREATHE_VERSION -- you'll hit __init__.py and therefore the same problem. This is a straightforward solution, but if long term you don't want to be doing things programmatically in setup.py then you'll ultimately have to undo this in the future.

Option 3: Require Sphinx for Installation

Worth mentioning explicitly, you can also just require your dependencies at install time. Not exactly ideal.

Personal Thoughts

I employ and therefore vote for Option 1 -- it's bulletproof, even if slightly awkward. Sphinx is the only thing that should be calling setup(app). And keeping the version number in exactly one location is highly desirable. Happy to PR if you want.

[2bndy5]

I employ and therefore vote for Option 1

I second this notion (despite the #noqa circumstance). Looks the my setuptools_scm idea isn't panning out in #766 review/discussion. In fact, I think #746 was aiming toward option 1 anyway, just without @svenevs expertise.

[michaeljones]

I don't know if it is ideal but I've merged a change to just double of the version number. We'll have to be more careful as maintainers but I've also added a CI check to make sure they stay in sync. I wish there was a more obviously better solution. I understand moving stuff into def setup would work and I had an old PR that does exactly that but it feels a bit awkward too. I'm happy enough seeing how this more basic approach works out.

I don't know much about importlib but if there are ways we can be a better member of the Python ecosystem from that perspective then I'm happy to explore those in future issues & PRs.