Skip to content

Comments

Improve long values in dict literals#3440

Merged
JelleZijlstra merged 10 commits intopsf:mainfrom
yilei:strcrash
Dec 15, 2022
Merged

Improve long values in dict literals#3440
JelleZijlstra merged 10 commits intopsf:mainfrom
yilei:strcrash

Conversation

@yilei
Copy link
Contributor

@yilei yilei commented Dec 14, 2022

Description

This PR changes the preview style:

Usually I'd like to keep the PR's scope minimal, but the lambda change & dict value change are depended by each other so it's hard to split into two PRs.

Checklist - did you ...

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

@github-actions
Copy link

github-actions bot commented Dec 14, 2022 •
edited
Loading

diff-shades results comparing this PR (ba1b968) to main (a282181). The full diff is available in the logs under the "Generate HTML diff report" step.

╭───────────────────────── Summary ─────────────────────────╮
│ 10 projects & 99 files changed / 1089 changes [+608/-481] │
│                                                           │
│ ... out of 2 355 274 lines, 10 989 files & 23 projects    │
╰───────────────────────────────────────────────────────────╯

Differences found.

What is this? | Workflow run | diff-shades documentation

@JelleZijlstra
Copy link
Collaborator

JelleZijlstra commented Dec 15, 2022

Notes from diff-shades:

             queryset = queryset.extra(
                 select={
                     "_prefetch_related_val_%s"
-                    % f.attname: "%s.%s"
-                    % (qn(join_table), qn(f.column))
+                    % f.attname: "%s.%s" % (qn(join_table), qn(f.column))
                     for f in fk.local_related_fields
                 }
             )

Note that both the key and the value in the dictcomp use % formatting. Looks a little weird, maybe we should put the key and value on separate lines. Seems out of scope for this PR though.

Pandas has a similarly odd-looking case with a + in the key:

         tests = [
             (
                 BusinessHour(),
                 {
                     Timestamp("2014-07-04 15:00")
-                    + Nano(5): Timestamp("2014-07-04 16:00")
-                    + Nano(5),
+                    + Nano(5): Timestamp("2014-07-04 16:00") + Nano(5),
                     Timestamp("2014-07-04 16:00")
-                    + Nano(5): Timestamp("2014-07-07 09:00")
-                    + Nano(5),
+                    + Nano(5): Timestamp("2014-07-07 09:00") + Nano(5),
                     Timestamp("2014-07-04 16:00")
-                    - Nano(5): Timestamp("2014-07-04 17:00")
-                    - Nano(5),
+                    - Nano(5): Timestamp("2014-07-04 17:00") - Nano(5),
                 },
             ),

Otherwise I like the changes from the PR: they make it easier to see when one dict key ends and the next one begins, because values get indented.

JelleZijlstra
JelleZijlstra reviewed Dec 15, 2022
View reviewed changes
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, just noticed two typos

@yilei yilei mentioned this pull request Dec 15, 2022
Open
yilei and others added 2 commits December 14, 2022 22:10
@yilei @JelleZijlstra
Update src/black/trans.py
6aa7d6e 
Co-authored-by: Jelle Zijlstra <[email protected]>
@yilei @JelleZijlstra
Update src/black/trans.py
ba1b968 
Co-authored-by: Jelle Zijlstra <[email protected]>
@yilei
Copy link
Contributor Author

yilei commented Dec 15, 2022

Thanks for the review! Agreed above two examples are out of scope from this PR, they concern the dict keys. Opened #3442 separately.

@JelleZijlstra JelleZijlstra merged commit 658c8d8 into psf:main Dec 15, 2022
@yilei yilei deleted the strcrash branch December 16, 2022 01:44
@yilei yilei mentioned this pull request Dec 16, 2022
Merged
3 tasks
JelleZijlstra pushed a commit that referenced this pull request Dec 16, 2022
@yilei
Fix an infinite recursion error exposed by #3440 (#3444)
7816393
@JelleZijlstra JelleZijlstra mentioned this pull request Dec 19, 2022
Closed
@yilei yilei mentioned this pull request Dec 19, 2022
Closed
@oskarkk oskarkk mentioned this pull request Jan 21, 2023
Open
yilei added a commit to yilei/black that referenced this pull request Feb 2, 2023
@yilei
Document the future style changes introduced in psf#3489 and psf#3440.
0a1f49d
ichard26 pushed a commit that referenced this pull request Feb 5, 2023
@yilei
Document the future style changes introduced in #3489 and #3440 (#3541)
ea5293b
@JelleZijlstra JelleZijlstra mentioned this pull request Jul 11, 2023
Closed
This was referenced Nov 2, 2023
Closed
Closed
@hauntsaninja hauntsaninja mentioned this pull request Nov 13, 2023
Closed
@konstin konstin mentioned this pull request Nov 14, 2023
Closed
28 tasks
@MichaReiser MichaReiser mentioned this pull request Nov 29, 2023
Closed
@dylwil3 dylwil3 mentioned this pull request Sep 19, 2025
Closed
31 tasks
@cobaltt7 cobaltt7 mentioned this pull request Nov 29, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

2 participants

@yilei @JelleZijlstra