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

Add the ability to skip checking some functions. #587

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mgilson
Copy link

@mgilson mgilson commented Apr 4, 2022

There are times when you might not want to check a function's
docstring. Specifically, we frequently want to ignore
test files. There is already an exemption that causes test
functions to be considered non-public. This extends on
that idea and makes it configurable so a user can cause
a test function to be allowed to have no docstring.

This is a potential implementation for #537, #515

Thanks for submitting a PR!

Please make sure to check for the following items:

  • Add unit tests and integration tests where applicable.
    If you've added an error code or changed an error code behavior,
    you should probably add or change a test case file under tests/test_cases/ and add
    it to the list under tests/test_definitions.py.
    If you've added or changed a command line option,
    you should probably add or change a test in tests/test_integration.py.
  • Add a line to the release notes (docs/release_notes.rst) under "Current Development Version".
    Make sure to include the PR number after you open and get one.

Please don't get discouraged as it may take a while to get a review.

There are times when you might not want to check a function's
docstring.  Specifically, we frequently want to ignore
test files.  There is already an exemption that causes test
functions to be considered non-public.  This extends on
that idea and makes it configurable so a user can cause
a test function to be allowed to have no docstring.
Copy link
Contributor

@aphedges aphedges left a comment

Choose a reason for hiding this comment

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

From looking at the code itself, this looks good except for a small concern. I'll try it out later this week to see if it works well.

name_skip = (
ignore_functions is not None
and not definition.docstring
and bool(ignore_functions.findall(definition.name))
Copy link
Contributor

Choose a reason for hiding this comment

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

search() is probably a better option than findall() because it only needs to find one instance of the pattern instead of all. However, function names should short enough that the performance difference is negligible, so I'm not sure a change is needed.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, I was just following along with the prior art for decorators (e.g. line 151).

I'm happy to change just this one (or both) to use pattern.search instead.

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

Successfully merging this pull request may close these issues.

4 participants