Skip to content

Fix formatting cells with magic methods and starting or trailing empty lines #4484

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.

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

Merged
merged 4 commits into from
Oct 14, 2024

Conversation

AleksMat
Copy link
Contributor

@AleksMat AleksMat commented Oct 14, 2024

Description

This PR fixes an issue where Jupyter cells containing any magic method and at least 1 empty line at the start or at the end of the cell don't get formatted at all.

This is an example of a cell that doesn't get formatted:



%time
a=1


But if there are no magic methods then formatting works. The following cell:



#No magic methods
a=1


gets correctly reformatted into:

# No magic methods
a = 1

It turns out the problem is in black.handle_ipynb_magics.mask_cell function:

  1. The call to TransformerManager.transform_cell returns a result without empty lines at the beginning of the cell. Because this part is implemented in ipython I didn't investigate why this is the case.

  2. The call to replace_magics returns a result without empty lines at the end of the cell. This is because the implementation of replace_magics assumes that

    "\n".join(src.splitlines())

    will always produce src again. This is not the case, e.g. "\n".join("\n\n".splitlines()) returns "\n".

  3. Because of 1. and 2. the check len(transformed.splitlines()) != len(src.splitlines()) is triggered and NothingChanged is raised.

Solution

  1. A comment about TransformerManager.transform_cell removing empty lines. Future improvements in ipython are possible.
  2. Minor update in replace_magics so that it won't be removing empty lines at the end anymore.
  3. The number of lines in transformed and src should be compared without starting and ending empty lines. Hence len(transformed.strip().splitlines()) != len(src.strip().splitlines())

I also added a few unit tests that test such cases.

Checklist - did you ...

  • Add an entry in CHANGES.md if necessary?
  • Add / update tests if necessary?
  • Add new / update outdated documentation?

Copy link
Collaborator

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@JelleZijlstra JelleZijlstra merged commit fff747d into psf:main Oct 14, 2024
46 checks passed
luketainton pushed a commit to luketainton/PwnedPW that referenced this pull request Feb 10, 2025
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [black](https://github.com/psf/black) ([changelog](https://github.com/psf/black/blob/main/CHANGES.md)) | dependency-groups | major | `<25.0.0,>=24.10.0` -> `<25.2.0,>=25.1.0` |

---

### Release Notes

<details>
<summary>psf/black (black)</summary>

### [`v25.1.0`](https://github.com/psf/black/blob/HEAD/CHANGES.md#2510)

[Compare Source](psf/black@24.10.0...25.1.0)

##### Highlights

This release introduces the new 2025 stable style ([#&#8203;4558](psf/black#4558)), stabilizing
the following changes:

-   Normalize casing of Unicode escape characters in strings to lowercase ([#&#8203;2916](psf/black#2916))
-   Fix inconsistencies in whether certain strings are detected as docstrings ([#&#8203;4095](psf/black#4095))
-   Consistently add trailing commas to typed function parameters ([#&#8203;4164](psf/black#4164))
-   Remove redundant parentheses in if guards for case blocks ([#&#8203;4214](psf/black#4214))
-   Add parentheses to if clauses in case blocks when the line is too long ([#&#8203;4269](psf/black#4269))
-   Whitespace before `# fmt: skip` comments is no longer normalized ([#&#8203;4146](psf/black#4146))
-   Fix line length computation for certain expressions that involve the power operator ([#&#8203;4154](psf/black#4154))
-   Check if there is a newline before the terminating quotes of a docstring ([#&#8203;4185](psf/black#4185))
-   Fix type annotation spacing between `*` and more complex type variable tuple ([#&#8203;4440](psf/black#4440))

The following changes were not in any previous release:

-   Remove parentheses around sole list items ([#&#8203;4312](psf/black#4312))
-   Generic function definitions are now formatted more elegantly: parameters are
    split over multiple lines first instead of type parameter definitions ([#&#8203;4553](psf/black#4553))

##### Stable style

-   Fix formatting cells in IPython notebooks with magic methods and starting or trailing
    empty lines ([#&#8203;4484](psf/black#4484))
-   Fix crash when formatting `with` statements containing tuple generators/unpacking
    ([#&#8203;4538](psf/black#4538))

##### Preview style

-   Fix/remove string merging changing f-string quotes on f-strings with internal quotes
    ([#&#8203;4498](psf/black#4498))
-   Collapse multiple empty lines after an import into one ([#&#8203;4489](psf/black#4489))
-   Prevent `string_processing` and `wrap_long_dict_values_in_parens` from removing
    parentheses around long dictionary values ([#&#8203;4377](psf/black#4377))
-   Move `wrap_long_dict_values_in_parens` from the unstable to preview style ([#&#8203;4561](psf/black#4561))

##### Packaging

-   Store license identifier inside the `License-Expression` metadata field, see
    [PEP 639](https://peps.python.org/pep-0639/). ([#&#8203;4479](psf/black#4479))

##### Performance

-   Speed up the `is_fstring_start` function in Black's tokenizer ([#&#8203;4541](psf/black#4541))

##### Integrations

-   If using stdin with `--stdin-filename` set to a force excluded path, stdin won't be
    formatted. ([#&#8203;4539](psf/black#4539))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4xNjQuMSIsInVwZGF0ZWRJblZlciI6IjM5LjE2NC4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJsaW50aW5nIl19-->

Reviewed-on: https://git.tainton.uk/repos/PwnedPW/pulls/283
Reviewed-by: Luke Tainton <[email protected]>
Co-authored-by: Renovate [BOT] <[email protected]>
Co-committed-by: Renovate [BOT] <[email protected]>
@AleksMat AleksMat deleted the fix-cells-empty-lines branch February 21, 2025 20:09
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.

2 participants