Remove Inclusion of Sphinx directives in deprecate_renamed_argument decorator - #5492

I think the failure is just a fluke - It seems I can't restart it myself, so could someone restart that build?

I wonder if this is something that can (or should) be addressed upstream, e.g. in astropy_helpers.sphinx.ext.numpydoc.py. So that directives have more freedom where they are located.
I also had an attempt replacing astropy_helper's numpydoc with sphinx.ext.napoleon (without this PR), in the hope that that would be up to date to handle directives inside a parameter list, but no luck either.

@evertrol Dynamically altered docstrings should be pretty rare and if one writes it by hand then one can put it where it (should) belong. Also I wouldn't like sphinx/numpydoc to move parts of the docstring around. That makes it somewhat non-deterministic.

The only thing I really miss in numpydoc is a History-section, where one could dump all the version-changed/added-stuff.

"then one can put it where it (should) belong.". I tried adding a .. warning:: or .. versionchanged:: manually, but with the same incorrect results. It seems that numpydoc really expects the Parameters section to be terminated by another section.

As for a History section, perhaps that is actually something that can be added to astropy_helper's numpydoc (if not further upstream)?

@evertrol You have to be careful about indentation when you add directives in parameters but it works. See for example copy-parameter in NDData - source

I'm not sure if it exactly works like this but in general this worked for me:

• leave one blank line between text and directive
• directive must have the same indentation as the text (for parameters it should match the indentation of the description text, not the parameter name).
• if the directive has additional text the text should be indented (for example)
• leave one blank line after the directive(-text).

@MSeifert04

directive must have the same indentation as the text (for parameters it should match the indentation of the description text, not the parameter name).

Thanks, that was the part I forgot. Probably because I was more focused on a general or section directive, not one related to a single parameter. (But it now has me wondering if the versionchanged directive should not be closer (directly next) to the overwrite parameter, as in the NDData example.)

But it now has me wondering if the versionchanged directive should not be closer (directly next) to the overwrite parameter

You mean removing the part that dynamically creates the docstring and just insert the directive manually?

But it now has me wondering if the versionchanged directive should not be closer (directly next) to the overwrite parameter

You mean removing the part that dynamically creates the docstring and just insert the directive manually?

More generally where such a note should be put: the NDData examples put an versionadded:: or versionchanged:: directly next to the relevant parameter. This PR & decorator put it near the top of the docstring. What do people prefer?

That may be done automatically (which requires some parsing of the docstring to infer the indentation level and change), or it may be done manually (which is essentially just an extra line similar to adding the @-decorator line, and a switch to turn off the dynamic update of the docstring).

My current preference would be next to the parameter and manually added, but perhaps there are good reasons not to (other than some repetition).

My current preference would be next to the parameter and manually added

I actually like that idea. It's a bit of repetition and it will likely be forgotten in some future cases but it's a) not removed from the docstring as soon as the decorator is removed and b) simplifies the decorator a lot.

I'll wait for others to chime in before I change anything though.

I would also prefer to have it next to the relevant parameters.

ok, I'll remove the dynamic adding of the directives, then #5171 or a follow-up PR needs to add the directives manually.

FYI: This only affects the dev-branch so the PR has no changelog entry.

@evertrol already changed #5171 so I think this PR is ready to be reviewed/merged.

Merging.

@pllim Thank you!

Linking to #5214 for future reference.