Skip to content
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.

Sign up for GitHub

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

Jump to bottom

chore: [pre-commit.ci] pre-commit autoupdate #2264

Merged
merged 7 commits into from
Aug 10, 2023
Merged

chore: [pre-commit.ci] pre-commit autoupdate #2264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
363350c
chore: [pre-commit.ci] pre-commit autoupdate
pre-commit-ci[bot] Aug 1, 2023
6790ace
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 1, 2023
b8faaa6
Apply additional_dependencies
matthewfeickert Aug 1, 2023
505753b
see if this patch fixes and if so then move this to a follow up PR
matthewfeickert Aug 10, 2023
ba052fa
Use mocker over unittest.mock
matthewfeickert Aug 10, 2023
9c634a3
Revert "Use mocker over unittest.mock"
matthewfeickert Aug 10, 2023
2ec8ee6
Revert "see if this patch fixes and if so then move this to a follow …
matthewfeickert Aug 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,21 @@ repos:
exclude: ^validation/|\.dtd$|\.xml$

- repo: https://github.com/astral-sh/ruff-pre-commit
rev: "v0.0.276"
rev: "v0.0.281"
hooks:
- id: ruff
args: ["--fix", "--show-fixes"]

- repo: https://github.com/psf/black-pre-commit-mirror
rev: 23.3.0
rev: 23.7.0
hooks:
- id: black-jupyter

- repo: https://github.com/asottile/blacken-docs
rev: 1.14.0
rev: 1.15.0
hooks:
- id: blacken-docs
additional_dependencies: [black==23.3.0]
additional_dependencies: [black==23.7.0]

- repo: https://github.com/pre-commit/mirrors-mypy
rev: v1.4.1
Expand All @@ -62,7 +62,7 @@ repos:
rev: 1.7.0
hooks:
- id: nbqa-ruff
additional_dependencies: [ruff==0.0.276]
additional_dependencies: [ruff==0.0.281]
args: ["--extend-ignore=F821,F401,F841,F811"]

- repo: https://github.com/codespell-project/codespell
Expand Down
2 changes: 1 addition & 1 deletion src/pyhf/contrib/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
# do tarfile.is_tarfile(BytesIO(response.content)).
# Instead, just use a 'try except' block to determine if the
# archive is a valid tarfile.
# TODO: Simplify after pyhf is Python 3.9+ only

Check notice on line 88 in src/pyhf/contrib/utils.py

View check run for this annotation

codefactor.io / CodeFactor

src/pyhf/contrib/utils.py#L88 

unresolved comment '# TODO: Simplify after pyhf is Python 3.9+ only' (C100)
try:
# Use transparent compression to allow for .tar or .tar.gz
with tarfile.open(
Expand Down Expand Up @@ -115,7 +115,7 @@
# directory up and then renamed as the name of the
# zipfile directory is set at zipfile creation time and
# isn't knowable in advance.
child_path = [child for child in output_directory.iterdir()][0]
child_path = next(iter(output_directory.iterdir()))

Check warning on line 118 in src/pyhf/contrib/utils.py

View check run for this annotation

Codecov / codecov/patch

src/pyhf/contrib/utils.py#L118 

Added line #L118 was not covered by tests
_tmp_path = output_directory.parent.joinpath(
Path(output_directory.name + "__tmp__")
)
Expand Down
4 changes: 2 additions & 2 deletions src/pyhf/contrib/viz/brazil.py
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

It is unclear to me why coverage is reporting the [0] case as being fully covered but the next case as only partial coverage. :?

Copy link
Member

Choose a reason for hiding this comment

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

Ah. Seems because help(next) shows

Help on built-in function next in module builtins:

next(...)
    next(iterator[, default])
    
    Return the next item from the iterator. If default is given and the iterator
    is exhausted, it is returned instead of raising StopIteration.

that nedbat/coveragepy#605 (comment) and nedbat/coveragepy#1617 might be relevant here.

Copy link
Member

@matthewfeickert matthewfeickert Aug 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@kratsg Is there a way to mock or do something else to get

pyhf/tests/contrib/test_viz.py

Lines 16 to 23 in 6c35247

def test_brazil_band_collection(datadir):
data = json.load(datadir.joinpath("hypotest_results.json").open(encoding="utf-8"))
fig = Figure()
ax = fig.subplots()
brazil_band_collection = brazil.plot_results(
data["testmus"], data["results"], test_size=0.05, ax=ax
)

to cause

pyhf/src/pyhf/contrib/viz/brazil.py

Lines 360 to 366 in 6c35247

# Order legend: ensure CLs expected band and test size are last in legend
handles, labels = ax.get_legend_handles_labels()
if not no_cls:
for label_part in ["exp", "pm1", "pm2", "alpha"]:
label_idx = [
idx for idx, label in enumerate(labels) if label_part in label
][0]

to raise a StopIteration when switched to using next? So to mock the value of labels such that one of the label_part values aren't found?

By construction the function should always get the values, so this might be an area where the dip in the test coverage by partial for 1 line is worth it.

Copy link
Member

@matthewfeickert matthewfeickert Aug 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, this can just be mocked and so can just be a follow up PR.

Original file line number Diff line number Diff line change
Expand Up @@ -361,9 +361,9 @@ def plot_results(test_pois, tests, test_size=0.05, ax=None, **kwargs):
handles, labels = ax.get_legend_handles_labels()
if not no_cls:
for label_part in ["exp", "pm1", "pm2", "alpha"]:
label_idx = [
label_idx = next(
idx for idx, label in enumerate(labels) if label_part in label
][0]
)
handles.append(handles.pop(label_idx))
labels.append(labels.pop(label_idx))

Expand Down
6 changes: 3 additions & 3 deletions tests/test_workspace.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ def test_join_items_outer_deep(join_items):
joined = pyhf.workspace._join_items(
'outer', left_items, right_items, key='name', deep_merge_key='deep'
)
assert [k['deep'] for k in joined if k['name'] == 'common'][0] == [
assert next(k['deep'] for k in joined if k['name'] == 'common') == [
{'name': 1},
{'name': 2},
]
Expand All @@ -389,7 +389,7 @@ def test_join_items_left_outer_deep(join_items):
joined = pyhf.workspace._join_items(
'left outer', left_items, right_items, key='name', deep_merge_key='deep'
)
assert [k['deep'] for k in joined if k['name'] == 'common'][0] == [
assert next(k['deep'] for k in joined if k['name'] == 'common') == [
{'name': 1},
{'name': 2},
]
Expand All @@ -400,7 +400,7 @@ def test_join_items_right_outer_deep(join_items):
joined = pyhf.workspace._join_items(
'right outer', left_items, right_items, key='name', deep_merge_key='deep'
)
assert [k['deep'] for k in joined if k['name'] == 'common'][0] == [
assert next(k['deep'] for k in joined if k['name'] == 'common') == [
{'name': 2},
{'name': 1},
]
Expand Down