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

Method to ignore portions of docstring [Community Feedback] #144

Open
mcarans opened this issue Jan 4, 2023 · 14 comments
Open

Method to ignore portions of docstring [Community Feedback] #144

mcarans opened this issue Jan 4, 2023 · 14 comments
Labels
C: stakeholder Relates to docformatter stakeholder requested behavior community feedback Issue is tracking feedback from the docformatter community P: enhancement Feature that is outside the scope of PEP 257 S: feedback Open but need feedback from OP to proceed with solution U: low A relatively low urgency issue
Milestone

Comments

@mcarans
Copy link

mcarans commented Jan 4, 2023

It would be useful to have a feature to tell docformatter to leave part of the docstring alone. For example, someone may have manually set up part of the docstring exactly as they want it and don't want it to be touched.

This issue follows from and continues the discussion here.

The proposal in that comment suggests adding # docfmt: off and # docfmt: on to the docstring. Assuming the same part of every docstring needs to be ignored, the problem is that it requires changing every docstring to add those comment directives which is not practical for a large project and there is the possibility that the directives will confuse other tools that parse the docstring. I think there needs to be a way to globally set whether docformatter ignores part of each docstring without needing to modify every docstring in a project.

@github-actions github-actions bot added the fresh This is a new issue label Jan 4, 2023
@weibullguy weibullguy added P: enhancement Feature that is outside the scope of PEP 257 C: stakeholder Relates to docformatter stakeholder requested behavior and removed fresh This is a new issue labels Jan 4, 2023
@weibullguy weibullguy added the S: feedback Open but need feedback from OP to proceed with solution label Jan 11, 2023
@weibullguy
Copy link
Member

weibullguy commented Jan 11, 2023

Issue #82 was requesting the same/similar functionality as this issue, so #82 was closed to this issue.

The solution proposed by the author of this issue is to use regex to ignore certain patterns. Two issues I have with this approach:

Assuming the same part of every docstring needs to be ignored

We can't make that assumption for every use case.

it requires changing every docstring to add those comment directives which is not practical

sed is your friend and can add the decorators to chunks of code based on the patterns you want to ignore. If there are multiple patterns you wanted to ignore, it could become unwieldly to manage.

The proposed solution would be to introduce decorators similar to other autoformatting tools like black and yapf:

  • # docfmt: skip turns off docformatter in-line
  • # docfmt: off and # docfmt: on turn docformatter off for a chunk of docstring and back on.

For example:

"""Short summary for the docstring that's actually really long and would be wrapped. # docfmt: skip

This is a long description.  It could be many paragraphs long.  Following
this long description, is non PEP-257 stuff like reST directives that we 
don't want docformatter to touch.

# docfmt: off
Args:
    url (str): URL or path to read from
# docfmt: on
"""

The only problem I encounter with the quick example is pydocstyle raising a D400: First line should end with a period (not 'p') warning. There could be others.

I'm going to pin this issue and ask the user base for feedback.

@weibullguy weibullguy pinned this issue Jan 11, 2023
@weibullguy weibullguy changed the title Set part of docstring to be ignored and not touched by docformatter Method to ignore portions of docstring [Community Feedback] Jan 11, 2023
@weibullguy weibullguy changed the title Method to ignore portions of docstring [Community Feedback] Method to ignore portions of docstring [Community Feedback Requested] Jan 11, 2023
@github-actions github-actions bot added the fresh This is a new issue label Jan 11, 2023
@weibullguy weibullguy changed the title Method to ignore portions of docstring [Community Feedback Requested] Method to ignore portions of docstring [Community Feedback] Jan 11, 2023
@mcarans
Copy link
Author

mcarans commented Jan 11, 2023

I can see use cases that would benefit from the fine grained control that decorators would give.

I think for my use case it would be better to wait for docformatter to support Google docstrings rather than change all the docstrings to add decorators to ignore Args: and below.

@weibullguy weibullguy removed the fresh This is a new issue label Jan 12, 2023
@ethiy
Copy link

ethiy commented Jan 12, 2023

The solution proposed by the author of this issue is to use regex to ignore certain patterns. Two issues I have with this approach:

Assuming the same part of every docstring needs to be ignored

We can't make that assumption for every use case.

it requires changing every docstring to add those comment directives which is not practical

sed is your friend and can add the decorators to chunks of code based on the patterns you want to ignore. If there are multiple patterns you wanted to ignore, it could become unwieldly to manage.

When a simple issue requires such an elaborate solution, users tend to ignore the tool in the first place. It is not maintainable nor practical.

I will be waiting for the second solution, you have proposed.

@weibullguy weibullguy added this to the v2.0.0 milestone Jan 23, 2023
@weibullguy weibullguy added the community feedback Issue is tracking feedback from the docformatter community label May 3, 2023
@electric-coder
Copy link

electric-coder commented Jun 1, 2023

I think it should be a priority to get to get correct wrapping for google style docstrings sections as reported in #174 . The linked issue doesn't allow refactoring of basic syntax; so introducing additional syntax as proposed by this FR will clutter the docstrings without solving essential problems.

For example, someone may have manually set up part of the docstring exactly as they want it and don't want it to be touched.

This should never be the case to begin with and the FR gives no practical example.

Considering the Napoleon-Sphinx configuration and autodoc extension configuration are feature rich most of the rendering is done by the documentation builder and any additional complexity is preferably exterior to the docstring and put in the reST files.

The only motivation I've seen mentioned consistently for keeping a field list or a docstring section with some formatting that "shouldn't be touched" (as said in this FR) is by users who want to document their code using sphinx.ext.autosummary with the main intent of automating their documentation generation without having to write custom reST. However, such documentation will either be necessarily very limited or soon finds itself having to write additional reST into the docstrings themselves besides the fields and the sections (a practice that certainly won't make for good docstrings).

Hence, while the FR seems reasonable at first glance I don't see a real need for it.

There's an additional problem, inserting additional markup into the docstring seems to solve a problem but there's no telling if it won't break any of the many 3rd party extensions out there that parse docstrings in their documentation build process. So any functionality gained by adding this feature to docformatter is likely to be unusable because it'll break other tools further down the workflow.

@weibullguy weibullguy added P: enhancement Feature that is outside the scope of PEP 257 and removed P: enhancement Feature that is outside the scope of PEP 257 labels Jun 7, 2023
@github-actions github-actions bot added the U: low A relatively low urgency issue label Jun 7, 2023
@weibullguy
Copy link
Member

weibullguy commented Jun 20, 2023

I think it should be a priority to get to get correct wrapping for google style docstrings sections as reported in #174

Support for Numpy and Google style docstrings are next up on the docket. Actually, I've already started working on that, but bugs take priority over new features.

This should never be the case to begin with and the FR gives no practical example.

I'm not sold on the use of decorators either, but issue #126 is a potential use case where a decorator might be the best approach. I don't think another argument (e.g., --ignore-module-docstrings) to docformatter would be the right solution.

There's an additional problem, inserting additional markup into the docstring seems to solve a problem but there's no telling if it won't break any of the many 3rd party extensions out there that parse docstrings in their documentation build process. So any functionality gained by adding this feature to docformatter is likely to be unusable because it'll break other tools further down the workflow.

I agree with you. I suspect a decorator could/would booger up pydocstyle as well.

@mcarans
Copy link
Author

mcarans commented Jun 20, 2023

The best solution would be Google docstring support. This was just a suggestion for an interim solution not knowing how long it would be before Google docstring support is completed. Happy to close this issue if the emphasis is now on adding Google docstring support. Presumably that would fix the handling of docstrings like this?

        """Returns header of tabular file pointed to by url and an iterator
        where each row is returned as a list or dictionary depending on the
        dict_form argument. The headers argument is either a row number or list
        of row numbers (in case of multi-line headers) to be considered as
        headers (rows start counting at 1), or the actual headers defined as a
        list of strings. It defaults to 1 and cannot be None. The dict_form
        arguments specifies if each row should be returned as a dictionary or a
        list, defaulting to a list.

        Optionally, headers can be inserted at specific positions. This is achieved blah blah blah blah blah blah
        using the header_insertions argument. If supplied, it is a list of tuples of the
        form (position, header) to be inserted. A function is called for each row. If
        supplied, it takes as arguments: headers (prior to any insertions) and row
        (which will be in dict or list form depending upon the dict_rows argument) and
        outputs a modified row or None to ignore the row.

        Args:
            url (str): URL or path to read from

@weibullguy
Copy link
Member

@mcarans Google and Numpy docstring support is in progress. Epytext and Sphinx styles are already supported. Let's keep this open since #126 looks like a possible use case for these decorators.

rytilahti added a commit to rytilahti/python-miio that referenced this issue Oct 21, 2023
Storing example responses in docstrings is dumb, but there is no nice way
to keep it on while avoiding formatting that makes them incomprehensible like:

-        """
-        Response of Yeelight Dual Control Module
-        {
-            'id': 1,
-            'result': [
-                {'did': 'switch_1_state', 'siid': 2, 'piid': 1, 'code': 0, 'value': False},
-                {'did': 'switch_1_default_state', 'siid': 2, 'piid': 2, 'code': 0, 'value': True},
-                {'did': 'switch_1_off_delay', 'siid': 2, 'piid': 3, 'code': 0, 'value': 300},
-                {'did': 'switch_2_state', 'siid': 3, 'piid': 1, 'code': 0, 'value': False},
-                {'did': 'switch_2_default_state', 'siid': 3, 'piid': 2, 'code': 0, 'value': False},
-                {'did': 'switch_2_off_delay', 'siid': 3, 'piid': 3, 'code': 0, 'value': 0},
-                {'did': 'interlock', 'siid': 4, 'piid': 1, 'code': 0, 'value': False},
-                {'did': 'flex_mode', 'siid': 4, 'piid': 2, 'code': 0, 'value': True},
-                {'did': 'rc_list', 'siid': 4, 'piid': 2, 'code': 0, 'value': '[{"mac":"9db0eb4124f8","evtid":4097,"pid":339,"beaconkey":"3691bc0679eef9596bb63abf"}]'},
-            ]
-        }
-        """
+        """Response of Yeelight Dual Control Module { 'id': 1, 'result': [ {'did':
+        'switch_1_state', 'siid': 2, 'piid': 1, 'code': 0, 'value': False}, {'did':
+        'switch_1_default_state', 'siid': 2, 'piid': 2, 'code': 0, 'value': True},
+        {'did': 'switch_1_off_delay', 'siid': 2, 'piid': 3, 'code': 0, 'value': 300},
+        {'did': 'switch_2_state', 'siid': 3, 'piid': 1, 'code': 0, 'value': False},
+        {'did': 'switch_2_default_state', 'siid': 3, 'piid': 2, 'code': 0, 'value':
+        False}, {'did': 'switch_2_off_delay', 'siid': 3, 'piid': 3, 'code': 0, 'value':
+        0}, {'did': 'interlock', 'siid': 4, 'piid': 1, 'code': 0, 'value': False},
+        {'did': 'flex_mode', 'siid': 4, 'piid': 2, 'code': 0, 'value': True}, {'did':
+        'rc_list', 'siid': 4, 'piid': 2, 'code': 0, 'value': '[{"mac":"9db0eb4124f8","ev
+        tid":4097,"pid":339,"beaconkey":"3691bc0679eef9596bb63abf"}]'}, ] }"""

Somewhat related to PyCQA/docformatter#144
@mcarans
Copy link
Author

mcarans commented Nov 27, 2023

I would still find it useful to be able to tell docformatter somehow to ignore Args: onwards in the absence of full Google docstring support. Perhaps the first pass of Google docstring support could be to simply ignore Args: and all that follows so that at least any paragraphs before Args: are wrapped correctly.

@electric-coder
Copy link

electric-coder commented Dec 2, 2023

@mcarans I've thought about this further and I think the most reasonable solution would be a pyproject.toml option specific to docformatter allowing to keep an exclusion list of modules/classes/attributes that docformatter will ignore, something like coverage.py's [tool.coverage.report] exclude_also = ["def __repr__",] for example.

Your proposal of excluding only the args section or the whole docstring could be two separate pyproject.toml sections. This would allow keeping the docformatter tool automatic at the minimal cost of maintaining an exclusion list. This approach would also have the advantage of allowing selective exclusion when bugs appear in the tool.

@aecorn
Copy link

aecorn commented Dec 6, 2023

Just wanted to add my two cents / question, since this is a "fresh issue".
Using numpy-style, im getting a lot of duplicate entries from methods being listed both in the class docstring and in their own docstrings. Using :noindex: Im loosing all the navigation to members in the sidebar.

Googled "ignore part of docstring" as I was hoping to have autodoc ignore the method-part of the class-docstring to maybe avoid this. Kinda want to keep it as documentation for the users about the classes.

Anyone got any workarounds for this, should I just delete the method-section of my class-docstring while I wait for something that can be used for this?

klass\classes\codes.py:docstring of klass.classes.codes.KlassCodes.change_dates:1: WARNING: duplicate object description of klass.classes.codes.KlassCodes.change_dates, other instance in reference, use :no-index: for one of them

@electric-coder
Copy link

electric-coder commented Dec 6, 2023

@aecorn the issue you're describing isn't related to docformatter but it's about how you write the docstrings and .rst files plus how you configure Sphinx.

WARNING: duplicate object description of

Google your warning on Stack Overflow as there are several possible configurations that can lead to it.

@mcarans
Copy link
Author

mcarans commented Dec 6, 2023

@mcarans I've thought about this further and I think the most reasonable solution would be a pyproject.toml option specific to docformatter allowing to keep an exclusion list of modules/classes/attributes that docformatter will ignore, something like coverage.py's [tool.coverage.report] exclude_also = ["def __repr__",] for example.

Your proposal of excluding only the args section or the whole docstring could be two separate pyproject.toml sections. This would allow keeping the docformatter tool automatic at the minimal cost of maintaining an exclusion list. This approach would also have the advantage of allowing selective exclusion when bugs appear in the tool.

For my use case to exclude Args and everything following, would that mean it looks like the following?

[tool.coverage.report] 
exclude_also = ["Args:",]

@electric-coder
Copy link

electric-coder commented Jan 15, 2024

@mcarans Assuming a pyproject.toml option would be the way to go... (I'm trying to look for comparable solutions in established libraries for inspiration!) When I referenced coverage.py's [tool.coverage.report] exclude_also = ["def __repr__",] this works well by picking up the declaration line and then excluding the subsequent block. However, your example is different because it's a docstring and not a Python declaration, so lets look at it:

For my use case to exclude Args and everything following, would that mean it looks like the following?

[tool.coverage.report] 
exclude_also = ["Args:",]

I think that since we're dealing with docstrings, the right solution would be somethings like Poetry's include and exclude, so for example where Poetry uses:

[tool.poetry]
# ...
include = [
    { path = "tests", format = "sdist" },
    { path = "for_wheel.txt", format = ["sdist", "wheel"] }
]

docformatter could use an exclusion list dictionary mapping qualified names (e.g. module.class) to docstring sections:

[tool.docformatter]
# ...
exclude = [
    {
     "moduleA.classB": ["Args", "Yields"],
     "moduleA.classB.methodC": ["Args",],
     "moduleD.functionE": ["Keyword Args", "Returns"],
     "moduleF": ["Returns",],
    }
]

This does come with one serious difficulty: It's likely not precise enough to bulk-exclude one docstring section by name in all modules. How to resolve the filename when there may be files with the same name?

I don't think it makes much sense using filenames but instead Python qualified names should be used. (However, a user may also want to format modules that aren't importable and aren't on sys.path?!) The approach taken by Python Sphinx is importing the modules to get to the __doc__ so there's certain to be no naming conflicts. However, this implies the non-negligible difficulty that docformatter would have to implement an import mechanism.

So what's the solution for this? Can we say that only importable objects should be formatted if included in pyproject.toml?

@mcarans
Copy link
Author

mcarans commented Jan 15, 2024

docformatter could use an exclusion list dictionary mapping qualified names (e.g. module.class) to docstring sections:

[tool.docformatter]
# ...
exclude = [
    {
     "moduleA.classB": ["Args", "Yields"],
     "moduleA.classB.methodC": ["Args",],
     "moduleD.functionE": ["Keyword Args", "Returns"],
     "moduleF": ["Returns",],
    }
]

This does come with one serious difficulty: It's likely not precise enough to bulk-exclude one docstring section by name in all modules. How to resolve the filename when there may be files with the same name?

I don't think it makes much sense using filenames but instead Python qualified names should be used. (However, a user may also want to format modules that aren't importable and aren't on sys.path?!) The approach taken by Python Sphinx is importing the modules to get to the __doc__ so there's certain to be no naming conflicts. However, this implies the non-negligible difficulty that docformatter would have to implement an import mechanism.

So what's the solution for this? Can we say that only importable objects should be formatted if included in pyproject.toml?

While this is optimal in allowing more fine grained per module exclusions, would it be quite a lot of work to implement? If so, then could the first iteration just do global exclusions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: stakeholder Relates to docformatter stakeholder requested behavior community feedback Issue is tracking feedback from the docformatter community P: enhancement Feature that is outside the scope of PEP 257 S: feedback Open but need feedback from OP to proceed with solution U: low A relatively low urgency issue
Projects
None yet
Development

No branches or pull requests

5 participants