fix building documentation out of source tree

kloczek commented 3 months ago

Move importing pyngrok after altering sys.path. This itrival fix allows buid documentation without have pyngrok installed using only source tree.

Description

A clear and concise description of what was changed.

Fixes # (issue number)

Type of Change

[ ] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] This change requires a schema change
[ ] This change requires a documentation update

Testing Done

A clear and concise description of the new tests added to validate the change as well as any manual testing done.

Checklist

[ ] I have added tests that prove my change works as expected, and make test passes with no new errors or warnings
[x] I have run make check to ensure no new errors or warnings
[ ] I have updated the docs, if applicable, and run make docs to ensure no new errors or warnings
[ ] I have performed a self-review of my own code
[ ] I have commented my code in particularly hard-to-understand areas

Without that patch documentation build fails with

+ /usr/bin/sphinx-build -n -T -b man docs build/sphinx/man
Running Sphinx v7.2.6

Traceback (most recent call last):
  File "/usr/lib/python3.9/site-packages/sphinx/config.py", line 358, in eval_config_file
    exec(code, namespace)  # NoQA: S102
  File "/home/tkloczko/rpmbuild/BUILD/pyngrok-7.1.5/docs/conf.py", line 8, in <module>
    from pyngrok import __version__
ModuleNotFoundError: No module named 'pyngrok'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/lib/python3.9/site-packages/sphinx/cmd/build.py", line 293, in build_main
    app = Sphinx(args.sourcedir, args.confdir, args.outputdir,
  File "/usr/lib/python3.9/site-packages/sphinx/application.py", line 211, in __init__
    self.config = Config.read(self.confdir, confoverrides or {}, self.tags)
  File "/usr/lib/python3.9/site-packages/sphinx/config.py", line 181, in read
    namespace = eval_config_file(filename, tags)
  File "/usr/lib/python3.9/site-packages/sphinx/config.py", line 371, in eval_config_file
    raise ConfigError(msg % traceback.format_exc()) from exc
sphinx.errors.ConfigError: There is a programmable error in your configuration file:

Traceback (most recent call last):
  File "/usr/lib/python3.9/site-packages/sphinx/config.py", line 358, in eval_config_file
    exec(code, namespace)  # NoQA: S102
  File "/home/tkloczko/rpmbuild/BUILD/pyngrok-7.1.5/docs/conf.py", line 8, in <module>
    from pyngrok import __version__
ModuleNotFoundError: No module named 'pyngrok'

Configuration error:
There is a programmable error in your configuration file:

Traceback (most recent call last):
  File "/usr/lib/python3.9/site-packages/sphinx/config.py", line 358, in eval_config_file
    exec(code, namespace)  # NoQA: S102
  File "/home/tkloczko/rpmbuild/BUILD/pyngrok-7.1.5/docs/conf.py", line 8, in <module>
    from pyngrok import __version__
ModuleNotFoundError: No module named 'pyngrok'

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 68.27%. Comparing base (9c56bee) to head (b1c2ac0).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #134 +/- ## =========================================== - Coverage 92.29% 68.27% -24.02% =========================================== Files 7 7 Lines 662 662 =========================================== - Hits 611 452 -159 - Misses 51 210 +159 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

alexdlaird commented 3 months ago

Thanks for fixing this! The auto-formatter moved this in the latest release, I'll merge this and also add a comment to ensure it doesn't get moved again.

kloczek commented 3 months ago

Thank you 👍 😄

alexdlaird commented 3 months ago

Do you need a new release to resolve your issue, or is this simply being in the main branch sufficient?

kloczek commented 3 months ago

No .. it is not-so-urgent change/it can wait 😋 I've submitted it to no longer maintain that patch in my rpm package (just flush of some minor changes 😄 )

alexdlaird commented 3 months ago

Running make docs also build the deps in the right order without this change and without error. Curious why this can't be used when building an RPM?

kloczek commented 3 months ago

Running make docs also build the deps in the right order without this change and without error. Curious why this can't be used when building an RPM?

Reason for that is very simple

[tkloczko@pers-jacek SPECS]$ grep ^%sphinx_build_man python-*spec | wc -l; ls -1 python-*spec | wc -l
692
1238

So .. as you see in +50% build procedures of python modules I'm using the same build procedure lego piece to build python module documentation as man page. Here is my rpm macro which I'm using to generate all those documentation:

%sphinx_build                   /usr/bin/sphinx-build
%sphinx_build_man() %{expand:\\\
        PBR_VERSION=%(v=%{version}; echo ${v%~*}) \\\
        PDM_BUILD_SCM_VERSION=%(v=%{version}; echo ${v%~*}) \\\
        SETUPTOOLS_SCM_PRETEND_VERSION=%(v=%{version}; echo ${v%~*}) \\\
        JARACO_PACKAGING_SPHINX_WHEEL=$(ls -1 $PWD/dist/*whl) \\\
        %sphinx_build -n -T -b man %["%{*}"?"%{*}":"docs"] build/sphinx/man}

Yes in many cases it requires some additional adjustments like this PR.

[tkloczko@pers-jacek SPECS]$ grep "Patch:.*%{name}-add_module_path_in_conf.py.patch" python-*spec | wc -l; grep "Patch:.*%{name}-fix_module_path_in_conf.py.patch" python-*spec| wc -l
244
21

In this case it is clearly visible that procedure which I'm using is possible to use in case +60% of of that population of +1.2k python modules which comes with sphinx rendered documentation .. without fixes like here 😋 Currently after testing everything on such scale I'm slowly creating PRs with those changes against each module. Nevertheless PR has been accepted so it decreased that add_module_path_in_conf patches counter by one 😋 (one more time .. thank you 😄)

alexdlaird commented 3 months ago

You will still see warnings with this approach, for example in type linking—for this package specifically, you'll see warnings related to pyyaml on the build if you don't install the package deps in pyproject.toml first. But the docs will still build and otherwise look fine, just without dependency links to other modules. Was just curious about your use case though, sounds like that's fine in your case as you primarily want it to build. Thanks for sharing!

kloczek commented 3 months ago

Here is sphinx output with this PR

+ /usr/bin/sphinx-build -n -T -b man docs build/sphinx/man
Running Sphinx v7.2.6
making output directory... done
loading intersphinx inventory from https://docs.python.org/3/objects.inv...
building [mo]: targets for 0 po files that are out of date
writing output...
building [man]: all manpages
updating environment: [new config] 4 added, 0 changed, 0 removed
reading sources... [100%] troubleshooting
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
writing... python-pyngrok.3 { api integrations troubleshooting } done
build succeeded.

The manual pages are in build/sphinx/man.

alexdlaird / pyngrok