style: Update to Black v20.8 formatting #1048

matthewfeickert · 2020-08-26T18:37:24Z

Description

The final beta releases of Black bring about some changes in formatting, as we see in Black v20.8b1. This PR applies those style changes, and appropriatley changes the pre-commit style change so that commits don't fail with Black conflicts.

This PR also introduces the use of pinning the rev used in pre-commit as this is advocated for by pre-commit core dev @asottile in psf/black#420 — thanks to @alexander-held and @henryiii for pointing this out and explaining more to us. (It seems from omry/omegaconf#340 that others are in a similar boat as we are right now). The dev docs are also updated to reflect this.

Checklist Before Requesting Reviewer

Tests are passing
"WIP" removed from the title of the pull request
Selected an Assignee for the PR to be responsible for the log summary

Before Merging

For the PR Assignees:

Summarize commit messages into a comprehensive review of the PR

* Apply Black v20.8b1 formatting
   - Should be similar to format of first stable release of Black
* Update Black pre-commit hook to use rev v20.8b1
   - Using pinned revs is the recommended approach for pre-commit: https://github.com/psf/black/issues/420
* Update development docs to advocate for and explain the use of pre-commit autoupdate

codecov · 2020-08-26T18:39:28Z

Codecov Report

Merging #1048 into master will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1048   +/-   ##
=======================================
  Coverage   96.76%   96.76%           
=======================================
  Files          59       59           
  Lines        3371     3371           
  Branches      478      478           
=======================================
  Hits         3262     3262           
  Misses         68       68           
  Partials       41       41

Flag	Coverage Δ
#unittests	`96.76% <100.00%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/pyhf/constraints.py	`96.94% <ø> (ø)`
src/pyhf/modifiers/histosys.py	`100.00% <ø> (ø)`
src/pyhf/modifiers/lumi.py	`100.00% <ø> (ø)`
src/pyhf/modifiers/normfactor.py	`100.00% <ø> (ø)`
src/pyhf/modifiers/normsys.py	`100.00% <ø> (ø)`
src/pyhf/modifiers/shapefactor.py	`100.00% <ø> (ø)`
src/pyhf/modifiers/shapesys.py	`100.00% <ø> (ø)`
src/pyhf/optimize/mixins.py	`100.00% <ø> (ø)`
src/pyhf/pdf.py	`96.39% <ø> (ø)`
src/pyhf/tensor/common.py	`100.00% <ø> (ø)`
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8fa2cc3...0b6afc0. Read the comment docs.

.pre-commit-config.yaml

This reverts commit 535d238.

kratsg · 2020-08-28T14:08:36Z

One thing might be to remove all trailing commas and see how black format handles this...

alexander-held · 2020-08-28T14:41:01Z

With trailing commas removed it changes quite a bit, e.g.

def extract_index_access(
    baseviewer, subviewer, indices,
):

(old black) becomes (with new black)

def extract_index_access(
    baseviewer,
    subviewer,
    indices,
):

but when you remove the comma after indices, it becomes (still new black)

def extract_index_access(baseviewer, subviewer, indices):

The trailing comma forces the split across lines no matter how long the line is I believe.

matthewfeickert · 2020-08-28T20:27:47Z

The trailing comma forces the split across lines no matter how long the line is I believe.

Ah this might be related to psf/black#1629 and psf/black#1649. Thanks for explaining that, @alexander-held. I'm going to put this back into Draft and think on this then.

matthewfeickert · 2020-08-29T06:51:47Z

One thing might be to remove all trailing commas and see how black format handles this...

With trailing commas removed it changes quite a bit, e.g.
...
The trailing comma forces the split across lines no matter how long the line is I believe.

Yeah, @alexander-held @kratsg I think you hit it on the head here. This PR is going to need some trailing comma removal before it can go in. Thanks to both of you for solving this for me.

matthewfeickert · 2020-08-29T07:12:43Z

src/pyhf/parameters/paramview.py

@@ -9,17 +9,15 @@ def _tensorviewer_from_parmap(par_map, batch_size):
    names, slices, _ = list(
        zip(
            *sorted(
-                [(k, v['slice'], v['slice'].start,) for k, v in par_map.items()],
+                [(k, v['slice'], v['slice'].start) for k, v in par_map.items()],


Note that the trailing comma is being removed here:

(k, v['slice'], v['slice'].start)

matthewfeickert · 2020-08-29T07:47:22Z

tests/test_tensor.py

-            tb.simple_broadcast(
-                tb.astensor([1, 1, 1]), tb.astensor([2]), tb.astensor([3, 3, 3])
-            ),
+    assert (


These blocks seem like they're really going to stay as is for the time being given things like psf/black#1629

matthewfeickert · 2020-08-29T07:49:07Z

@kratsg @lukasheinrich Ready for another review. @alexander-held @henryiii your reviews are of course welcome too. :)

matthewfeickert added 3 commits August 26, 2020 13:26

Apply new black to src

bf10783

Apply black to tests

0e50cd1

Reapply Black to src

0193627

matthewfeickert added style Changes that do not affect the meaning of the code chore Other changes that don't modify src or test files labels Aug 26, 2020

matthewfeickert self-assigned this Aug 26, 2020

Update rev tag in pre-commit

535d238

matthewfeickert changed the title ~~style: Update to new version of Black~~ style: Update to Black v20.8 formatting Aug 28, 2020

matthewfeickert marked this pull request as ready for review August 28, 2020 03:17

matthewfeickert requested review from kratsg and lukasheinrich August 28, 2020 03:17

matthewfeickert commented Aug 28, 2020

View reviewed changes

.pre-commit-config.yaml Show resolved Hide resolved

Revert "Update rev tag in pre-commit"

776b8c7

This reverts commit 535d238.

kratsg approved these changes Aug 28, 2020

View reviewed changes

matthewfeickert marked this pull request as draft August 28, 2020 20:27

Use pre-commit autoupdate to bump black rev

6faf167

Update dev docs to explain pre-commit autoupdate

7f55eb4

matthewfeickert added the docs Documentation related label Aug 29, 2020

matthewfeickert added 2 commits August 29, 2020 02:08

Fix issues introduced by trailing commas

d5e8e6c

Also grab this one

f1242b5

matthewfeickert commented Aug 29, 2020

View reviewed changes

Fix issues introduced by trailing commas for tests

e8d34ed

matthewfeickert mentioned this pull request Aug 29, 2020

"INTERNAL ERROR: Black produced different code on the second pass of the formatter" psf/black#1629

Closed

Also get tensorviewer

8e823e1

matthewfeickert commented Aug 29, 2020

View reviewed changes

matthewfeickert requested a review from kratsg August 29, 2020 07:47

matthewfeickert marked this pull request as ready for review August 29, 2020 07:47

matthewfeickert requested a review from henryiii August 29, 2020 07:48

Get missed test_optim

0b6afc0

kratsg approved these changes Aug 31, 2020

View reviewed changes

matthewfeickert merged commit e3f50bb into master Aug 31, 2020

matthewfeickert deleted the chore/black-update branch August 31, 2020 22:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

style: Update to Black v20.8 formatting #1048

style: Update to Black v20.8 formatting #1048

matthewfeickert commented Aug 26, 2020 •

edited

Loading

codecov bot commented Aug 26, 2020 •

edited

Loading

kratsg commented Aug 28, 2020

alexander-held commented Aug 28, 2020 •

edited

Loading

matthewfeickert commented Aug 28, 2020

matthewfeickert commented Aug 29, 2020

matthewfeickert Aug 29, 2020

matthewfeickert Aug 29, 2020

matthewfeickert commented Aug 29, 2020

style: Update to Black v20.8 formatting #1048

style: Update to Black v20.8 formatting #1048

Conversation

matthewfeickert commented Aug 26, 2020 • edited Loading

Description

Checklist Before Requesting Reviewer

Before Merging

codecov bot commented Aug 26, 2020 • edited Loading

Codecov Report

kratsg commented Aug 28, 2020

alexander-held commented Aug 28, 2020 • edited Loading

matthewfeickert commented Aug 28, 2020

matthewfeickert commented Aug 29, 2020

matthewfeickert Aug 29, 2020

Choose a reason for hiding this comment

matthewfeickert Aug 29, 2020

Choose a reason for hiding this comment

matthewfeickert commented Aug 29, 2020

matthewfeickert commented Aug 26, 2020 •

edited

Loading

codecov bot commented Aug 26, 2020 •

edited

Loading

alexander-held commented Aug 28, 2020 •

edited

Loading