Skip to content

Conversation

Andrea-Oliveri
Copy link

Good afternoon,
this is a continuation of @lizawang's pull request titled Feature align assignment, PR#1030 .

I have addressed all the PR comments by @bwendling.

Of note:

  • I have renamed the knob NEW_ALIGNMENT_AFTER_COMMENTLINE to ALIGN_ASSIGNMENT_RESTART_AFTER_COMMENTS, as recommended.
  • I have refactored the function _AlignAssignment into multiple smaller functions.
    • Here, it is worth clarifying that since _AlignAssignment was heavily, heavily inspired from _AlignTrailingComments, I decided to group the common logic into a single generic function which takes callbacks for the specific tasks.
    • I believe this makes the generic function already more readable but I am not going to pretend I fully understand the logic in the callbacks.
    • I simply understood what they try to achieve, choose appropriate names for the callback functions, copy-pasted the relevant parts into new functions, and run the tests to ensure no regressions occurred.
    • If this is considered unnecessary work, I can revert _AlignTrailingComments to its previous state and we can keep duplicated logic.
  • I have solved the formatting issues in that review.
  • I have added new tests for edge cases.
  • I have solved conflicts with current main branch.

Copied here, the description of PR#1030 with minor changes:



The knobs

The main functions for all the knobs are added in reformatter.py.

knob: ALIGN_ASSIGNMENT

This is an option to align the assignment and augmented assignment operators of different variable assignment lines in the same block.

For the case when there are two assignment operators on one line, only the first assignment operator is aligned:

timeend   = record[ "timeend" ] = ts_end
self      = "xyz"
tablename = "treditab"

New block starts with new alignment. There are 5 cases that are featured to indicate a new block:
If there is any blank line in between:

a   = 1
abc = 2

b  = 3
bc = 4

If there is any comment line in between:

a   = 1
abc = 2
# comment
b  = 3
bc = 4

If there is any if/ while/ for line or function definition def line in between:

a   = 1
abc = 2
if condition == None:
    var = ''
b  = 3
bc = 4

If there is any object(e.g. a dictionary, a list, a argument list, a set, a function call) that with its items on newlines after '=' in between:

tablename = "treditab"
list      = [
    {
        "field"    : "ediid",
        "type"     : "text",
        "required" : True,
        "html"     : {},
    }
]
test = "another block"

If there is any variable assignment lines with different indentation in between:

a   = 1
abc = 2
if condition == None:
    var       += ''
    var_long  -= 4
b  = 3
bc = 4

knob: ALIGN_ASSIGNMENT_RESTART_AFTER_COMMENTS

This option is made under the consideration that some programmers may prefer to remain the alignment with comment lines inside.

If it is set False, a comment line in between won't create a new alignment block. Entries before and after the comment line/lines will align with their assignment operators.

Preparations

To make the alignment functions possible, some other changes are made to other files.

  • format_token.py: necessary boolean functions are added, is_assign, is_augassign.

Finally

In the end, the knobs are added into the style.py and tests for each one are also updated in yapftests folder.

  • style.py: all the 2 knobs are set to be False by default.
  • Unitests are added into reformatter_basic_test.py.

jamesderlin and others added 30 commits July 12, 2021 17:59
"... split such that all elements are on a single line." could be
interpreted to mean that all elements will be combined into a single
line, but the knob really means that each element will appear on a
separate line.
* support ignore config in pyproject.toml

* add doc in readme

* add tests

* changelog
Replace if statement with if expression
Replace multiple comparisons with in operator
…ngle blank line (google#954)

* PEP8: Method definitions inside a class are surrounded by a single blank line.

* Add changelog entry for "pep8" style update
* [RFC] Added yapf_api.FormatTree

The new entry point allows to avoid re-parsing when a lib2to3 pytree
has already been constructed.

Closes google#951.

* Updated CHANGELOG

* Fix spacing

Co-authored-by: Bill Wendling <[email protected]>
…oogle#962)

This was previously done by ``yapf_api.FormatCode``, but I think it makes
more sense to have it in ``pytree_utils.ParseCodeToTree``, because the
grammar requires a trailing EOL.
Don't rely upon the original "pytree node" object. This helps isolate
the lib2to3 objects from yapf.
AkashKumarSingh11032001 and others added 23 commits March 19, 2024 18:40
Bumps [actions/checkout](https://github.com/actions/checkout) from 4.1.1 to 4.1.2.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@b4ffde6...9bb5618)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
….2 (google#1213)

Bumps [peter-evans/create-pull-request](https://github.com/peter-evans/create-pull-request) from 6.0.0 to 6.0.2.
- [Release notes](https://github.com/peter-evans/create-pull-request/releases)
- [Commits](peter-evans/create-pull-request@b1ddad2...70a41ab)

---
updated-dependencies:
- dependency-name: peter-evans/create-pull-request
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…1218)

Bumps [actions/setup-python](https://github.com/actions/setup-python) from 5.0.0 to 5.1.0.
- [Release notes](https://github.com/actions/setup-python/releases)
- [Commits](actions/setup-python@0a5c615...82c7e63)

---
updated-dependencies:
- dependency-name: actions/setup-python
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* ci.yml: Disable "fail fast"

.. to better see if failure depends on architecture and/or Python
version only

* ci.yml: Stop CI from trying Python 3.7 on arm64 macOS

Symptom was this error:
> Error: The version '3.7' with architecture 'arm64' was not found for macOS 14.6.1.
> The list of all available versions can be found here: https://raw.githubusercontent.com/actions/python-versions/main/versions-manifest.json

Note that Python 3.7 is end-of life since 2023-06-27.
Bumps [actions/checkout](https://github.com/actions/checkout) from 4.1.2 to 4.2.0.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@9bb5618...d632683)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…r.load (google#1243)

Previously, bad timing could make another process run into reading a
half-written pickle cache file, and thus fail like this:

> Traceback (most recent call last):
>   File "[..]/bin/yapf", line 5, in <module>
>     from yapf import run_main
>   File "[..]/lib/python3.11/site-packages/yapf/__init__.py", line 41, in <module>
>     from yapf.yapflib import yapf_api
>   File "[..]/lib/python3.11/site-packages/yapf/yapflib/yapf_api.py", line 38, in <module>
>     from yapf.pyparser import pyparser
>   File "[..]/lib/python3.11/site-packages/yapf/pyparser/pyparser.py", line 44, in <module>
>     from yapf.yapflib import format_token
>   File "[..]/lib/python3.11/site-packages/yapf/yapflib/format_token.py", line 23, in <module>
>     from yapf.pytree import pytree_utils
>   File "[..]/lib/python3.11/site-packages/yapf/pytree/pytree_utils.py", line 30, in <module>
>     from yapf_third_party._ylib2to3 import pygram
>   File "[..]/lib/python3.11/site-packages/yapf_third_party/_ylib2to3/pygram.py", line 29, in <module>
>     python_grammar = driver.load_grammar(_GRAMMAR_FILE)
>                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   File "[..]/lib/python3.11/site-packages/yapf_third_party/_ylib2to3/pgen2/driver.py", line 252, in load_grammar
>     g.load(gp)
>   File "[..]/lib/python3.11/site-packages/yapf_third_party/_ylib2to3/pgen2/grammar.py", line 95, in load
>     d = pickle.load(f)
>         ^^^^^^^^^^^^^^
> EOFError: Ran out of input
….5 (google#1239)

Bumps [peter-evans/create-pull-request](https://github.com/peter-evans/create-pull-request) from 6.0.2 to 7.0.5.
- [Release notes](https://github.com/peter-evans/create-pull-request/releases)
- [Commits](peter-evans/create-pull-request@70a41ab...5e91468)

---
updated-dependencies:
- dependency-name: peter-evans/create-pull-request
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/checkout](https://github.com/actions/checkout) from 4.2.0 to 4.2.1.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@d632683...eef6144)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Copy link

google-cla bot commented Aug 13, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@Andrea-Oliveri
Copy link
Author

Apologies, apparently I spelled incorrectly my email which causes cla check to fail. I will put in draft until solved.

@Andrea-Oliveri Andrea-Oliveri marked this pull request as draft August 13, 2025 15:42
@Andrea-Oliveri Andrea-Oliveri force-pushed the feature_align_assignment branch from 1b636a8 to 53f2f5f Compare August 13, 2025 16:00
@Andrea-Oliveri
Copy link
Author

I broke the history. Will try different approach to solve wrong email and open a new PR. Apologies

@Andrea-Oliveri Andrea-Oliveri deleted the feature_align_assignment branch August 13, 2025 16:08
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.