Chilipp / docrep

A Python Module for intelligent reuse of docstrings
Apache License 2.0
30 stars 4 forks source link

Unable to remove multiple params using docrep.delete_params() #29

Open mitches-got-glitches opened 3 years ago

mitches-got-glitches commented 3 years ago

Hello, thanks for this package - I'm finding it really helpful.

I'm not sure if this is a feature request, a bug, or unclear documentation, but I'm having trouble removing more than one parameter from the DocstringProcessor.

From the documentation:

The new docstring without the selected parts will be accessible as base_key + '.no_' + '|'.join(params), e.g. 'original_key.no_param1|param2'.

Here's my workflow so far.

import docrep
docstrings = docrep.DocstringProcessor()

@docstrings.get_sections(
    base='base_values',
    sections=docstrings.param_like_sections,
)
def my_base_func():
    ...

docstrings.delete_params('base_values.parameters', 'method')
# Make a copy because we don't want to delete 'base_period' from the main DocstringProcesser.
core_params = copy(docstrings)
core_params.delete_params('base_values.parameters', 'base_period')

@core_params.dedent
def new_func():
    """My new func.

    Parameters
    ------------
    %(base_values.parameters.no_base_period|method)s
    """
    ...

So my issue is that I'm following the docs to try and remove both parameters "base_period" and "method" but I've had no luck in doing that using the workflow described above. If it's an issue in my implementation rather than a bug, then a clearer example in the docs for dropping multiple parameters would be much appreciated.

Additional point:

Thanks

Chilipp commented 3 years ago

Hey @mitches-got-glitches !

In your case, you are modifying the do string twice, so the correct code is something like

docstrings.delete_params('base_values.parameters', 'method')
docstrings.delete_params('base_values.parameters.no_method', 'base_period')

And then access it through the 'base_values.parameters.no_method.no_base_period' key.

Or you do something like

docstrings.delete_params('base_values.parameters', 'method', 'base_period')

and access it via 'base_values.parameters.no_method|base_period'

Having to first do .delete_params() and then add extra syntax in the docstring seems like duplication of work. What's stopping configuring the API so the user doesn't have to even call .delete_params() on the docstring processor, instead just specifying which parameters they want to keep directly in the docstring such as "%(base_values.parameters.param1|param2)s".

I see your point. There should be an inplace parameter for the delete_params method. Would you like to contribute a PR?

mitches-got-glitches commented 3 years ago

Thanks for explaining @Chilipp, that's very useful. I can certainly consider a PR, it might be a couple of weeks before I get a chance to dig into the source code. Would you like me to modify this issue to a feature request, or close and open a new one once I can describe the work to be done a bit better?

Chilipp commented 3 years ago

hi @mitches-got-glitches! no worries, I'll take care of it, it's really just a minor thing :smiley: I'll let you know

We can keep this issue here open