Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Fix numpy d410 #233

Merged
merged 11 commits into from Feb 27, 2017
3 changes: 2 additions & 1 deletion docs/release_notes.rst
Expand Up @@ -20,7 +20,8 @@ New Features
* Decorator-based skipping via ``--ignore-decorators`` has been added (#204).
* Support for using pycodestyle style wildcards has been added (#72, #209).
* Superfluous opening quotes are now reported as part of D300 (#166, #225).
* Support for ``numpy`` conventions verification has been added (#129).
* Support for ``numpy`` conventions verification has been added (#129, #226).
* Fixed a false-positive recognition of `D410` and added `D412` (#230, #233).

Bug Fixes

Expand Down
74 changes: 54 additions & 20 deletions src/pydocstyle/checker.py
Expand Up @@ -4,7 +4,7 @@
import string
import sys
import tokenize as tk
from itertools import takewhile
from itertools import takewhile, tee
from re import compile as re
from collections import namedtuple

Expand All @@ -14,6 +14,11 @@
Method, Function, NestedFunction, Parser, StringIO)
from .utils import log, is_blank

try:
from itertools import zip_longest
except ImportError:
from itertools import izip_longest as zip_longest


__all__ = ('check', )

Expand All @@ -34,6 +39,16 @@ def decorator(f):
return decorator


def pairwise(iterable, default_value):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this to utils.py.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

"""Return pairs of items from `iterable`.

pairwise([1, 2, 3], default_value=None) -> (1, 2) (2, 3), (3, None)
"""
a, b = tee(iterable)
_ = next(b, default_value)
return zip_longest(a, b, fillvalue=default_value)


class ConventionChecker(object):
"""Checker for PEP 257 and numpy conventions.

Expand Down Expand Up @@ -462,15 +477,15 @@ def _is_a_docstring_section(context):

@classmethod
def _check_section_underline(cls, section_name, context, indentation):
"""D4{07,08,09,10}, D215: Section underline checks.
"""D4{07,08,09,12}, D215: Section underline checks.

Check for correct formatting for docstring sections. Checks that:
* The line that follows the section name contains
dashes (D40{7,8}).
* The amount of dashes is equal to the length of the section
name (D409).
* The line that follows the section header (with or without dashes)
is empty (D410).
* The section's content does not begin in the line that follows
the section header (D412).
* The indentation of the dashed line is equal to the docstring's
indentation (D215).
"""
Expand All @@ -486,8 +501,8 @@ def _check_section_underline(cls, section_name, context, indentation):

if not dash_line_found:
yield violations.D407(section_name)
if next_non_empty_line_offset == 0:
yield violations.D410(section_name)
if next_non_empty_line_offset > 0:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

next_non_empty_line_offset is a confusing name. How about something like blank_lines_after_header?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Legit :^)

yield violations.D412(section_name)
else:
if next_non_empty_line_offset > 0:
yield violations.D408(section_name)
Expand All @@ -498,22 +513,24 @@ def _check_section_underline(cls, section_name, context, indentation):
section_name,
len(dash_line.strip()))

line_after_dashes = \
context.following_lines[next_non_empty_line_offset + 1]
if not is_blank(line_after_dashes):
yield violations.D410(section_name)

if leading_space(dash_line) > indentation:
yield violations.D215(section_name)

if next_non_empty_line_offset + 1 < len(context.following_lines):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, this means "if this is not the last section", right? Please add a comment to explain this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed some of the logic here. The method is a bit long, but I don't think there's a non-artificial way of splitting it to several ones. Tell me if you think I should split it anyways.

line_after_dashes = \
context.following_lines[next_non_empty_line_offset + 1]
if is_blank(line_after_dashes):
yield violations.D412(section_name)

@classmethod
def _check_section(cls, docstring, definition, context):
"""D4{05,06,11}, D214: Section name checks.
"""D4{05,06,10,11}, D214: Section name checks.

Check for valid section names. Checks that:
* The section name is properly capitalized (D405).
* The section is not over-indented (D214).
* The section name has no superfluous suffix to it (D406).
* There's a blank line after the section (D410).
* There's a blank line before the section (D411).

Also yields all the errors from `_check_section_underline`.
Expand All @@ -532,6 +549,10 @@ def _check_section(cls, docstring, definition, context):
if suffix:
yield violations.D406(capitalized_section, context.line.strip())

if (not context.following_lines or
not is_blank(context.following_lines[-1])):
yield violations.D410(capitalized_section)

if not is_blank(context.previous_line):
yield violations.D411(capitalized_section)

Expand All @@ -549,13 +570,12 @@ def check_docstring_sections(self, definition, docstring):

Short Summary
-------------

This is my summary.

Returns
-------

None.

'''

Section names appear in `SECTION_NAMES`.
Expand All @@ -580,18 +600,32 @@ def _suspected_as_section(_line):
SectionContext = namedtuple('SectionContext', ('section_name',
'previous_line',
'line',
'following_lines'))
'following_lines',
'original_index'))

# First - create a list of possible contexts. Note that the
# `following_linex` member is until the end of the docstring.
contexts = (SectionContext(self._get_leading_words(lines[i].strip()),
lines[i - 1],
lines[i],
lines[i + 1:])
lines[i + 1:],
i)
for i in suspected_section_indices)

for ctx in contexts:
if self._is_a_docstring_section(ctx):
for err in self._check_section(docstring, definition, ctx):
yield err
# Now that we have manageable objects - rule out false positives.
contexts = (c for c in contexts if self._is_a_docstring_section(c))

# Now we shall trim the `following lines` field to only reach the
# next section name.
for a, b in pairwise(contexts, None):
end = -1 if b is None else b.original_index
new_ctx = SectionContext(a.section_name,
a.previous_line,
a.line,
lines[a.original_index + 1:end],
a.original_index)
for err in self._check_section(docstring, definition, new_ctx):
yield err


parse = Parser()
Expand Down
2 changes: 2 additions & 0 deletions src/pydocstyle/violations.py
Expand Up @@ -223,6 +223,8 @@ def to_rst(cls):
'Expected {0!r} dashes in section {1!r}, got {2!r}')
D410 = D4xx.create_error('D410', 'Missing blank line after section', '{0!r}')
D411 = D4xx.create_error('D411', 'Missing blank line before section', '{0!r}')
D412 = D4xx.create_error('D412', 'Section content should be in the line '
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest making the description here clearer: "No blank lines allowed between a section header and its content".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

'following its header', '{0!r}')


class AttrDict(dict):
Expand Down
35 changes: 33 additions & 2 deletions src/tests/test_cases/sections.py
Expand Up @@ -17,6 +17,7 @@ def not_capitalized():

returns
-------
A value of some sort.

"""

Expand All @@ -29,6 +30,7 @@ def superfluous_suffix():

Returns:
-------
A value of some sort.

"""

Expand All @@ -39,10 +41,21 @@ def no_underline():
"""Valid headline.

Returns
A value of some sort.

"""


@expect(_D213)
@expect("D407: Missing dashed underline after section ('Returns')")
@expect("D410: Missing blank line after section ('Returns')")
def no_underline():
"""Valid headline.

Returns
"""


@expect(_D213)
@expect("D408: Section underline should be in the line following the "
"section's name ('Returns')")
Expand All @@ -52,6 +65,7 @@ def blank_line_before_underline():
Returns

-------
A value of some sort.

"""

Expand All @@ -64,6 +78,7 @@ def bad_underline_length():

Returns
--
A value of some sort.

"""

Expand All @@ -75,7 +90,7 @@ def no_blank_line_after_section():

Returns
-------
A whole lot of values.
A value of some sort.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the "blank line after a section" should not be enforced for the last section of the docstring (as discussed in #230).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@farmersez what did you end up doing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I split it to a different error code which is not raised by default in the numpy convention.

"""


Expand All @@ -87,6 +102,7 @@ def no_blank_line_before_section():
The function's description.
Returns
-------
A value of some sort.

"""

Expand All @@ -98,6 +114,7 @@ def section_overindented():

Returns
-------
A value of some sort.

"""

Expand All @@ -109,10 +126,22 @@ def section_underline_overindented():

Returns
-------
A value of some sort.

"""


@expect(_D213)
@expect("D215: Section underline is over-indented (in section 'Returns')")
@expect("D410: Missing blank line after section ('Returns')")
def section_underline_overindented_and_contentless():
"""Valid headline.

Returns
-------
"""


@expect(_D213)
def ignore_non_actual_section():
"""Valid headline.
Expand All @@ -132,21 +161,23 @@ def ignore_non_actual_section():
def section_name_in_first_line():
"""Returns
-------
A value of some sort.

"""


@expect(_D213)
@expect("D405: Section name should be properly capitalized "
"('Short Summary', not 'Short summary')")
@expect("D412: Section content should be in the line following its header "
"('Short Summary')")
@expect("D409: Section underline should match the length of its name "
"(Expected 7 dashes in section 'Returns', got 6)")
@expect("D410: Missing blank line after section ('Returns')")
@expect("D411: Missing blank line before section ('Raises')")
@expect("D406: Section name should end with a newline "
"('Raises', not 'Raises:')")
@expect("D407: Missing dashed underline after section ('Raises')")
@expect("D410: Missing blank line after section ('Raises')")
def multiple_sections():
"""Valid headline.

Expand Down