Black bindings #6951
Merged
Black bindings #6951
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These are treated as warnings by extract_pyi, so they don't stop the build process.
dhalbert
requested changes
Sep 27, 2022
There was a problem hiding this comment.
I think this is a good idea, since it canonicalizes the documentation.
One thing I do not understand is the removal of blank lines. Are you running black on the entire extraction of //| lines in each .c file?
Here's a typical set of changes, where blank lines between the defs are removed. This is not what black would do if they were all in one file:
Running black manually on an extract of SleepMemory.c:
class SleepMemory:
def __init__(self) -> None:
"""Not used. Access the sole instance through `alarm.sleep_memory`."""
...
def __bool__(self) -> bool:
"""``sleep_memory`` is ``True`` if its length is greater than zero.
This is an easy way to check for its existence.
"""
...
def __len__(self) -> int:
"""Return the length. This is used by (`len`)"""
...It ensures there are blank lines between the defs.
|
From @jepler in discord:
|
jepler
force-pushed
the
black-bindings
branch
from
c2b2096 to
bc9a65d
Compare
|
I run each 'section' through black, so black has no section-to-section memory to draw on. After Dan's earlier comment I revised the script so that it includes a blank "//|" at the end of each block of "//|". This reduces the churn somewhat, in raw lines of code. Here are the differences to the generated stubs from this branch: |
jepler
force-pushed
the
black-bindings
branch
from
bc9a65d to
0cc47ab
Compare
|
I revised this further, based on discord discussions:
|
Since black_bindings.py will pass each contiguous "//|"-block to black independently, they must each be a fully formed Python item.
jepler
force-pushed
the
black-bindings
branch
from
0cc47ab to
3a0d237
Compare
|
.. updated again to ditch the trailing blank lines. |
dhalbert
approved these changes
Sep 27, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As an alternative to #6950 arrange to run black on each "//|" stubs block within bindings and shared-bindings, so that the formatting is made consistent at every commit.
Reasons not to do this right now: It churns a LOT and I was not attentive enough to review everything.