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

📔✅ Update Docstring Coverage check #892

Merged
merged 50 commits into from
May 4, 2022
Merged

📔✅ Update Docstring Coverage check #892

merged 50 commits into from
May 4, 2022

Conversation

mberr
Copy link
Member

@mberr mberr commented Apr 26, 2022

This PR updates the coverage check to check all files by default. It also adds explicit ignores where appropriate, and missing docstring otherwise.

I used a (hacky) script to add some # docstr-coverage: inherited in deae930, which I add here for reference.

import pathlib
from typing import List


def process_lines(lines: List[str]):
    i = 0
    while i < len(lines):
        if "# noqa: D102" in lines[i]:
            # search backward to def
            j = i
            while j >= 0 and not lines[j].lstrip().startswith("def"):
                j -= 1
            # extract indention of def
            def_idx = lines[j].find("def")
            # look at line before
            need_docstr = True
            if j > 0:
                need_docstr = "# docstr-coverage: inherited" not in lines[j - 1]
                # do not consider annotated functions
                need_docstr = need_docstr and not "@" in lines[j - 1]
            if need_docstr:
                lines.insert(j, " " * def_idx + "# docstr-coverage: inherited")
                i = j - 1
        i += 1
    yield from lines
    yield ""


root = pathlib.Path("src", "pykeen")
for path in root.rglob("*.py"):
    old = path.read_text()
    new = "\n".join(process_lines(old.splitlines()))
    if new != old:
        path.write_text(new)
        print(path)

@mberr
Copy link
Member Author

mberr commented May 2, 2022

the only remaining error should be fixed by #893

@mberr mberr marked this pull request as ready for review May 2, 2022 15:39
@mberr mberr changed the title Update Docstring Coverage check 📔✅ Update Docstring Coverage check May 2, 2022
@mberr mberr requested a review from cthoyt May 3, 2022 08:52
super().__init__(p=p, power_norm=power_norm)

# docstr-coverage: inherited
Copy link
Member

Choose a reason for hiding this comment

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

why is this here twice

Copy link
Member Author

Choose a reason for hiding this comment

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

@@ -964,6 +1040,7 @@ class SEInteraction(
func = pkf.se_interaction

@staticmethod
# docstr-coverage: inherited
Copy link
Member

Choose a reason for hiding this comment

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

probably an artifact from the automated script, right? it's weird this is after the decorator

Copy link
Member Author

Choose a reason for hiding this comment

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

super().__init__(p=p, power_norm=power_norm)

@staticmethod
# docstr-coverage: inherited
Copy link
Member

Choose a reason for hiding this comment

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

weird order

Copy link
Member Author

Choose a reason for hiding this comment

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

super().__init__(p=p, power_norm=power_norm)

@staticmethod
# docstr-coverage: inherited
Copy link
Member

Choose a reason for hiding this comment

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

weird order

Copy link
Member Author

Choose a reason for hiding this comment

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

@mberr mberr merged commit 4815be6 into master May 4, 2022
@mberr mberr deleted the docstr-cov branch May 4, 2022 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants