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

Add separate conf.py for the different docs types #12900

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

matthewhughes934
Copy link
Contributor

@matthewhughes934 matthewhughes934 commented Aug 6, 2024

The goal here is to allow building the man pages without all the
additional requirements needed for the html docs, so that e.g.
developers packaging pip can also package the man pages with minimal
dependencies.

Specifically, this should have no impact on the build output of nox -s docs, but does allow:

sphinx-build \
    -c docs/man \
    -d docs/build/doctrees/man \
    -b man \
    docs/man \
    docs/build/man

to run with only sphinx dependency (in addition to pips
dependencies). While doing this I also used long-opts to sphinx-build
in the noxfile since I found this more understandable at a glance

An __init__.py was added under docs/man to satisfy mypy. If this
wasn't added it would error:

docs/man/conf.py: error: Duplicate module named "conf" (also at
"docs/html/conf.py")
docs/man/conf.py: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#mapping-file-paths-to-modules for more info
docs/man/conf.py: note: Common resolutions include: a) using `--exclude` to avoid checking one of them, b) adding `__init__.py` somewhere, c) using `--explicit-package-bases` or adjusting MYPYPATH
Found 1 error in 1 file (errors prevented further checking)

Adding an extra __init__.py under docs/html would result in a
separate error since then the html module is local (and not the stdlib
one) resulting in a error from sphinx-build:

Extension error:
Could not import extension sphinx.builders.linkcheck (exception: No module named 'html.parser')

Issue: #12881

Comment on lines 31 to 34
copyright = copyright
project = project
version = version
release = release
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternatively: add these to __all__ in docs/common_conf.py and import * from there, but I preferred being explicit

docs_dir = os.path.dirname(os.path.dirname(__file__))
sys.path.insert(0, docs_dir)

from common_conf import copyright, project, release, version # noqa: E402
Copy link
Member

Choose a reason for hiding this comment

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

You can use __all__ to signify the values are re-exported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can use __all__ to signify the values are re-exported.

Are you suggesting add them to __all__ in this file, instead of the assignments below? Or add these to __all__ in common_conf and just import * here?

Copy link
Member

Choose a reason for hiding this comment

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

Why not simply ignore the Ruff rule?

Suggested change
from common_conf import copyright, project, release, version # noqa: E402
from common_conf import copyright, project, release, version # noqa: E402, F401

Are you suggesting add them to __all__ in this file, instead of the assignments below?

Names declared in __all__ become part of the module's interface and thus Ruff won't delete unused imports that populate those names. AFAIU, this is what @uranusjr was suggesting. I think this is even less obvious to a reader.

Alternatively we could do from common_config import version as version for each configuration option to declare them as re-exports.

from common_conf import copyright as copyright  # noqa: E402
from common_conf import project as project  # noqa: E402
from common_conf import release as release  # noqa: E402
from common_conf import version as version  # noqa: E402

But this seems verbose IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why not simply ignore the Ruff rule?

No reason not to, see c04ff52

Copy link
Member

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

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

You need to install the HTML dependencies in the docs-live nox session, otherwise it will break.

I don't like excluding the configuration files from mypy, but I also can't think of anything better...

docs_dir = os.path.dirname(os.path.dirname(__file__))
sys.path.insert(0, docs_dir)

from common_conf import copyright, project, release, version # noqa: E402
Copy link
Member

Choose a reason for hiding this comment

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

Why not simply ignore the Ruff rule?

Suggested change
from common_conf import copyright, project, release, version # noqa: E402
from common_conf import copyright, project, release, version # noqa: E402, F401

Are you suggesting add them to __all__ in this file, instead of the assignments below?

Names declared in __all__ become part of the module's interface and thus Ruff won't delete unused imports that populate those names. AFAIU, this is what @uranusjr was suggesting. I think this is even less obvious to a reader.

Alternatively we could do from common_config import version as version for each configuration option to declare them as re-exports.

from common_conf import copyright as copyright  # noqa: E402
from common_conf import project as project  # noqa: E402
from common_conf import release as release  # noqa: E402
from common_conf import version as version  # noqa: E402

But this seems verbose IMO.

@matthewhughes934
Copy link
Contributor Author

You need to install the HTML dependencies in the docs-live nox session, otherwise it will break.

Thanks, I completely missed that one, fixed a08f157

@matthewhughes934 matthewhughes934 marked this pull request as ready for review August 10, 2024 11:01
Copy link
Member

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

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

Actually, it looks like adding a __init__.py to only the man directory is a valid workaround.

@matthewhughes934
Copy link
Contributor Author

Actually, it looks like adding a __init__.py to only the man directory is a valid workaround.

ah, good idea, trying this: 20cbe13

@@ -31,6 +31,7 @@ repos:
rev: v1.10.0
hooks:
- id: mypy
# exclude due to errors from duplicate conf.py under docs
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# exclude due to errors from duplicate conf.py under docs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤦 dab08d1

The goal here is to allow building the `man` pages without all the
additional requirements needed for the `html` docs, so that e.g.
developers packaging `pip` can also package the man pages with minimal
dependencies.

Specifically, this should have no impact on the build output of `nox -s
docs`, but does allow:

    sphinx-build \
        -c docs/man \
        -d docs/build/doctrees/man \
        -b man \
        docs/man \
        docs/build/man

to run with only `sphinx` dependency (in addition to `pip`s
dependencies). While doing this I also used long-opts to `sphinx-build`
in the `noxfile` since I found this more understandable at a glance

An `__init__.py` was added under `docs/man` to satisfy `mypy`. If this
wasn't added it would error:

    docs/man/conf.py: error: Duplicate module named "conf" (also at
    "docs/html/conf.py")
    docs/man/conf.py: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#mapping-file-paths-to-modules for more info
    docs/man/conf.py: note: Common resolutions include: a) using `--exclude` to avoid checking one of them, b) adding `__init__.py` somewhere, c) using `--explicit-package-bases` or adjusting MYPYPATH
    Found 1 error in 1 file (errors prevented further checking)

Adding an extra `__init__.py` under `docs/html` would result in a
separate error since then the `html` module is local (and not the stdlib
one) resulting in a error from `sphinx-build`:

    Extension error:
    Could not import extension sphinx.builders.linkcheck (exception: No module named 'html.parser')

Issue: pypa#12881
@ichard26 ichard26 added this to the 24.3 milestone Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants