sourcery-ai / sourcery

Instant AI code reviews
https://sourcery.ai
MIT License
1.51k stars 65 forks source link

False positive for __init__.py : Modules should have docstrings #323

Closed Josverl closed 1 year ago

Josverl commented 1 year ago

Description

Warnings / informationals are generated for init.py that are used as a module/package placeholder ℹ️ Sourcery - Modules should have docstrings

image

Code snippet that reproduces issue

an empty __init__.py

Debug Information

sourcery, version 1.0.4

IDE Version: Version: 1.75.0 (user setup) Commit: e2816fe719a4026ffa1ee0189dc89bdfdbafb164 Date: 2023-02-01T15:23:45.584Z Electron: 19.1.9 Chromium: 102.0.5005.194 Node.js: 16.14.2 V8: 10.2.154.23-electron.0 OS: Windows_NT x64 10.0.22621 Sandboxed: No

Josverl commented 1 year ago

Quick-Fix: Add init.py to the exclusions for this rule

- id: docstrings-for-modules
  pattern: |
    '''!!!'''
    ...
  condition: pattern.in_module_scope()
  paths:
    exclude:
    - test_*.py
    - '*_test.py'
    - '__init__.py'
Hellebore commented 1 year ago

Hi! We flag this in __init__.py files to mirror exactly the Google Python Style Guide - see here: https://google.github.io/styleguide/pyguide.html#381-docstrings

It refers to packages requiring docstrings, which I think are what would go into these.

Since this set of rules is meant to be a faithful implementation of the guide I'd rather not exclude them altogether.

I am (very) sympathetic to not wanting these flagged however. I think what I will do is split the check for docstrings in init files into a separate docstrings-for-packages rule. This can then be more easily individually disabled without having to override the rule.

Josverl commented 1 year ago

The vast majority of the __init__.py files I write are empty, because many packages don't have anything to initialize. so perhaps skip the requirement for init.py files that are shorter than 2 bytes

as your reference starts with Python uses docstrings to document code. and an empty file dos not contain code , therefore should not contain a docstring reading """ there is no code here """"

(but bard may indicate differently ....)

Hellebore commented 1 year ago

Hi @Josverl - I think the docstring here is meant to document the entirety of the package rather than the init file itself.

See PEP 257: All modules should normally have docstrings, and all functions and classes exported by a module should also have docstrings. Public methods (including the init constructor) should also have docstrings. A package may be documented in the module docstring of the init.py file in the package directory.

As of the latest release you should be able to disable this rule individually by adding docstrings-for-packages to your list of disabled rules, so I'll now close this.