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

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

Open
mitches-got-glitches opened this issue Aug 10, 2021 · 3 comments

Comments

@mitches-got-glitches
Copy link

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:

  • 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".

Thanks

@Chilipp
Copy link
Owner

Chilipp commented Aug 10, 2021

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
Copy link
Author

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
Copy link
Owner

Chilipp commented Aug 11, 2021

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

We can keep this issue here open

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants