[merge with Python 3.9 ~ October 2024] Preserve argument defaults to Sphinx documentation

[emollier]

Description

This change improves Sphinx documentation reproducibility by preserving argument defaults.

Indeed, James Addison noted that:

the hostname of the build host appears in some of the documentation built by Sphinx and the autodoc extension. The hostname is evaluated from a class attribute that calls the Python socket.gethostname method.

In more detail:

The TaskVineManagerConfig dataclass includes an address attribute that is set to the value of socket.gethostname() when the class is loaded.

Meanwhile, the TaskVineExecutor.__init__ method manager_config argument has a default value of a no-args constructed TaskVineManagerConfig instance.

When Sphinx builds documentation, by default it will emit a Python repr() of the manager_config argument, causing the hostname of the build host to be included.

We can solve that by instructing the Sphinx autodoc extension to retain the textual representation of argument lists as they are found in the source code, instead of evaluated and repr'd equivalents.

Changed Behaviour

From James:

[The patch] resolves the problem by enabling the Sphinx autodoc extension autodoc_preserve_defaults configuration setting. Please note that this does affect other output in the documentation.

I thus must highlight, that if you prefer documentation with results of interpretation instead of the raw variable names, and if you are uninterested in reproducible artifacts building, then you may want to reject the present merge request.

Fixes

This patch has originally been written to address the Debian bug #1063542. James originally planned to forward the patch, but I haven't seen it in the set of open or closed pull requests, so I guess they may have been busy on other fronts.

Type of change

• Bug fix
• Update to human readable text: Documentation

Have a nice day, :)
Étienne.

[benclifford]

For some context: We addressed that non-reproducibility issue in a different way, PR #3066 because it turns out that field in Task Vine doesn't need to be that way at all. Waiting for another dev to pick it up and approve it, though. That PR probably deprioritised upstreaming this particular change for @jayaddison

I think this #3225 PR is broadly a good thing to do too, but the reason I ended up with the #3066 approach is because I encountered this awkward chain of events:

Some awkwardnesses in the dependency chain:

When I try this with Sphinx 7.1.x I encounter this bug, which issues a
warning (and possibly is not outputting documentation correctly - I haven't
checked):

https://github.com/sphinx-doc/sphinx/issues/11767

That's fixed in Sphinx 7.2.x.

Our CI builds with SPHINXOPTS=-W so that we don't accumulate warnings that
no human sees - so this would cause a build error.

Sphinx 7.2.x does not support Python 3.8, so we're constrained to Sphinx
7.1.x

and Parsl is planned to work with Python 3.8 until the end of life dates in
https://devguide.python.org/versions/ which is October 2024.

[benclifford]

I think it would be fine to leave this open until we drop 3.8 support, then merge it with an upgraded sphinx too. When #3066 gets merged, I think/hope the specific issue https://bugs.debian.org/1063542 will be resolved by that too.

[emollier]

Hi Ben,

How come I missed #3066? Thanks for the context, I'll keep an eye on it, and will recheck how things go if it gets approved. In the meantime rebasing the patch on Debian side as versions go should not be too problematic (I knew way worse).

Have a nice day, :)
Étienne.

[benclifford]

tag @NishchayKarle who is doing some "which versions should we support?" work

[benclifford]

Once PR #3629 is merged, the next stage, upgrading sphinx, can happen.