PEP8-format generated revisions

Issue #307 new
Adrian
created an issue

Even though they usually need to be modified a bit, it would be nice if the generated statements were PEP8-valid by default (besides maybe line-length..)

Comments (9)

  1. jmagnusson

    What about having one argument per line of a function call in the generated code? Should fix most problems of non PEP-8 compliant code. Example with what I mean:

    op.alter_column(
            'blocki18n', 'links_to_text',
            existing_type=sa.TEXT(),
            server_default='',
            nullable=False
        )
    
  2. Adrian reporter

    Exactly. The most common indentation in that case is this though:

    op.alter_column(
        'blocki18n',
        'links_to_text',
        existing_type=sa.TEXT(),
        server_default='',
        nullable=False
    )
    
  3. ukrutt

    UPDATE: oops, sorry, saw now that this code has changed in script.py.mako -- however when I tested this I had failed to update my own old mako file. Please disregard...

    please disregard comments below...

    It looks like a fair amount of work was done on this leading up to https://bitbucket.org/zzzeek/alembic/commits/tag/rel_0_8_9. I still get a few pep8 warnings though,

    project/alembic/versions/175a74063cca_test.py:15:1: E402 module level import not at top of file
    project/alembic/versions/175a74063cca_test.py:16:1: E402 module level import not at top of file
    

    These are due to this code in the top of the generated file:

    # revision identifiers, used by Alembic.
    revision = '175a74063cca'
    down_revision = '99c454c52997'
    branch_labels = None
    depends_on = None
    
    from alembic import op
    import sqlalchemy as sa
    

    (specifically, the two imports.) It seems like this is a fairly straightforward thing to fix; however I'm new to this code in particular (and autogenerated code in general...)

  4. Wes Downey

    Here is another example:

    def upgrade():
        # ### commands auto generated by Alembic - please adjust! ###
        op.create_table('table_name',
        sa.Column('id', sa.Integer(), nullable=False),
    

    This fails linting for these two rules:

    • E128 continuation line under-indented for visual indent
    • E122 continuation line missing indentation or outdented

    The passing format would be like this:

    def upgrade():
        # ### commands auto generated by Alembic - please adjust! ###
        op.create_table(
            'table_name',
            sa.Column('id', sa.Integer(), nullable=False),
    
  5. mike waites

    Hey

    I've recently started helping Mike out with sqlalchemy and alembic. On first inspection I agree with @Adrian

    op.alter_column(
        'blocki18n',
        'links_to_text',
        existing_type=sa.TEXT(),
        server_default='',
        nullable=False
    )
    

    Would solve the vast majority of peps -- That said there might be things I'm over looking. I'll get started on this on Tuesday and try and get a couple of "worst case scenario" tests cases together that highlight the issues. From there, the rendering of op instructions is fairly centralised so it should be fairly straight forward to prove out.

    The work here will certainly be trying to produce test cases that produce all of the formatting issues. Perhaps it would make sense to deal with the really obvious ones first rather than going to deep.

    It would be a great help if anyone interested in this fix could list any pep errors they have encountered not already listed above.

    I'll report back here with my findings. 🤘

  6. Log in to comment