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 building documentation out of source tree #134

Merged
merged 1 commit into from
Mar 17, 2024
Merged

fix building documentation out of source tree #134

merged 1 commit into from
Mar 17, 2024

Conversation

kloczek
Copy link
Contributor

@kloczek kloczek commented Mar 17, 2024

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
  • 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'

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

Signed-off-by: Tomasz Kłoczko <[email protected]>
Copy link

codecov bot commented Mar 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

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

Additional details and impacted files
@@             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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alexdlaird
Copy link
Owner

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.

@alexdlaird alexdlaird self-requested a review March 17, 2024 21:10
@alexdlaird alexdlaird merged commit 823dccd into alexdlaird:main Mar 17, 2024
14 of 15 checks passed
@kloczek
Copy link
Contributor Author

kloczek commented Mar 17, 2024

Thank you 👍 😄

@alexdlaird
Copy link
Owner

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

@kloczek
Copy link
Contributor Author

kloczek commented Mar 18, 2024

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
Copy link
Owner

alexdlaird commented Mar 18, 2024

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
Copy link
Contributor Author

kloczek commented Mar 18, 2024

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
Copy link
Owner

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
Copy link
Contributor Author

kloczek commented Mar 18, 2024

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.

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