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.

Sign up for GitHub

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

Jump to bottom

D103 linter warnings from script.py.mako templates #1567

Open
peterjc opened this issue Nov 5, 2024 · 4 comments · May be fixed by #1568
Open

D103 linter warnings from script.py.mako templates #1567

peterjc opened this issue Nov 5, 2024 · 4 comments · May be fixed by #1568
Labels
migration environment

Comments

@peterjc
Copy link
Contributor

peterjc commented Nov 5, 2024

Describe the bug

Scripts created under <project>/alembic/versions/<name>.py from the Mako template fail the pydocstyle D103 style check, e.g. via flake8 or ruff check:

Expected behavior

The templates would include minimal docstrings for the upgrade and downgrade functions.

To Reproduce

Followed the tutorial, having enabled ruff check in alembic.ini and then run this with a trailing dot in the message to avoid D400 or D415.

$ alembic revision -m "Original schema."

Or without using this via the configuration, follow that with:

$ ruff check --select D103  */alembic/versions/*.py
xxx/alembic/versions/7c4c8748a5e0_original_schema.py:39:5: D103 Missing docstring in public function
   |
39 | def upgrade() -> None:
   |     ^^^^^^^ D103
40 |     pass
   |

xxx/alembic/versions/7c4c8748a5e0_original_schema.py:43:5: D103 Missing docstring in public function
   |
43 | def downgrade() -> None:
   |     ^^^^^^^^^ D103
44 |     pass
   |

Found 2 errors.

Error

# Copy error here. Please include the full stack trace.

Versions.

  • OS: macOS
  • Python: 3.12
  • Alembic: 1.14.0
  • SQLAlchemy: 2.0.35
  • Database: Sqlite
  • DBAPI:

Additional context

PR to follow.

Have a nice day!
@peterjc peterjc added the requires triage New issue that requires categorization label Nov 5, 2024
peterjc added a commit to peterjc/alembic that referenced this issue Nov 5, 2024
@peterjc
Avoid D103 linter warnings via script.py.mako
fa3bcd5 
This will silence the commonly used pydocstyle D103 warnings
from flake8 or ruff check:

http://www.pydocstyle.org/en/stable/error_codes.html#grouping
https://docs.astral.sh/ruff/rules/undocumented-public-function/

Closes sqlalchemy#1567
@peterjc peterjc linked a pull request Nov 5, 2024 that will close this issue
Open
3 tasks
@zzzeek zzzeek added migration environment and removed requires triage New issue that requires categorization labels Nov 5, 2024
@zzzeek
Copy link
Member

zzzeek commented Nov 5, 2024

hi -

I disagree with these linter rules and I would disable them. you can change your own script.py.mako to resolve. feel free to send a pull request otherwise I would close this.

@peterjc
Copy link
Contributor Author

peterjc commented Nov 5, 2024

I agree not everyone will use these rules, and they can be disabled on a file by file basis if the rest of the project does.

However, the PR is simple and small, and will mean less work for those alembic users which do run with the docstring linter rules enabled.

@CaselIT
Copy link
Member

CaselIT commented Nov 5, 2024

I'm personally -1 here. The suggested doc strings in the PR don't add anything in my point of view.
As mike mentioned alembic init is a one time thing and you are supposed to customize your env.py and make template to fit your project.

Up to you mike, but I would close this and not merge the PR

@zzzeek
Copy link
Member

zzzeek commented Nov 5, 2024

I think it's harmless to have a basic docstring in the upgrade/downgrade method. I think a little bit of putting a more complete description in the docstring as to how the code gets there and that it should be edited etc. but then I dont konw that I want a huge paragraph being spit out there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
migration environment
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Avoid D103 linter warnings via script.py.mako peterjc/alembic
3 participants
@peterjc @zzzeek @CaselIT