Skip to content

Fix duplication of docs in extend_command #282

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

czgdp1807
Copy link
Contributor

@czgdp1807 czgdp1807 commented May 31, 2025

Originally reported in scipy/scipy#23041 (comment)

extend_command duplicates doc strings of commands decorated with extend_command. Whatever commands I have tried in SciPy, all have docs from the original spin function and the SciPy function of the command. Take a look at the following examples,

spin shell --help (in SciPy),

(scipy-dev) 20:05:56:~/Quansight/scipy % spin shell --help
Usage: spin shell [OPTIONS] [SHELL_ARGS]...

  💻 Launch shell with PYTHONPATH set

  SHELL_ARGS are passed through directly to the shell, e.g.:

  spin shell -- -c 'echo $PYTHONPATH'

  Ensure that your shell init file (e.g., ~/.zshrc) does not override the PYTHONPATH.

  💻 Launch shell with PYTHONPATH set

  SHELL_ARGS are passed through directly to the shell, e.g.:

  spin shell -- -c 'echo $PYTHONPATH'

  Ensure that your shell init file (e.g., ~/.zshrc) does not override the PYTHONPATH.

spin docs --help (in SciPy)

(scipy-dev) 20:06:38:~/Quansight/scipy % spin docs --help 
Usage: spin docs [OPTIONS] [SPHINX_TARGET]

  📖 Build Sphinx documentation

  By default, SPHINXOPTS="-W", raising errors on warnings. To build without raising on warnings:

    SPHINXOPTS="" spin docs

  To list all Sphinx targets:

    spin docs targets

  To build another Sphinx target:

    spin docs TARGET

  📖 Build Sphinx documentation

  By default, SPHINXOPTS="-W", raising errors on warnings. To build without raising on warnings:

    SPHINXOPTS="" spin docs

  To list all Sphinx targets:

    spin docs targets

  To build another Sphinx target:

    spin docs TARGET

  E.g., to build a zipfile of the html docs for distribution:

    spin docs dist

I think the problem is with the following line,

my_cmd.help = (my_cmd.help or "") + "\n\n" + (user_func.__doc__ or "")

If both, my_cmd.help and user_func.__doc__ will be present then both will be combined. In my opinion, better idea would be to give priority to user_func.__doc__ if it is present. Otherwise, fallback to my_cmd.help.

In this PR, the outputs of the above commands look like as follows,

Usage: spin shell [OPTIONS] [SHELL_ARGS]...

  💻 Launch shell with PYTHONPATH set

  SHELL_ARGS are passed through directly to the shell, e.g.:

  spin shell -- -c 'echo $PYTHONPATH'

  Ensure that your shell init file (e.g., ~/.zshrc) does not override the PYTHONPATH.
.
.
.
(scipy-dev) 20:18:54:~/Quansight/scipy % spin docs --help   
Usage: spin docs [OPTIONS] [SPHINX_TARGET]

  📖 Build Sphinx documentation

  By default, SPHINXOPTS="-W", raising errors on warnings. To build without raising on warnings:

    SPHINXOPTS="" spin docs

  To list all Sphinx targets:

    spin docs targets

  To build another Sphinx target:

    spin docs TARGET

  E.g., to build a zipfile of the html docs for distribution:

    spin docs dist

czgdp1807 added 2 commits May 31, 2025 20:23
Signed-off-by: Gagandeep Singh <gdp.1807@gmail.com>
Signed-off-by: Gagandeep Singh <gdp.1807@gmail.com>
@czgdp1807 czgdp1807 marked this pull request as ready for review June 1, 2025 10:20
@rgommers
Copy link
Contributor

rgommers commented Jun 1, 2025

This actually seems to be covered already by the doc keyword to extend_command:

spin/spin/cmds/util.py

Lines 128 to 130 in fee6abe

doc : str
Replacement docstring.
The wrapped function's docstring is also appended.

Did you try using that to get the desired docstring contents?

@rgommers
Copy link
Contributor

rgommers commented Jun 1, 2025

So according to that documentation, adding doc="" should already remove the spin docs, and only pick up whatever is in the docstring of the command in the package itself.

@czgdp1807
Copy link
Contributor Author

czgdp1807 commented Jun 1, 2025

Yes. Makes sense. However isn't that confusing? I think, it should be, doc = "" by default in spin? Because as someone extending the command, I would like the documentation string I am providing to override the spin documentation by default. Any reasons for current behaviour of spin?

cc: @stefanv

@czgdp1807 czgdp1807 marked this pull request as draft June 1, 2025 12:06
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