Skip to content

Conversation

@kratsg
Copy link
Contributor

@kratsg kratsg commented Aug 10, 2022

Description

Handle cases where channel is missing data in the XML configuration (associated with Asimov). c.f. Issue #1911

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
* Raise RuntimeError for cases where channel is missing data in the XML configuration
(associated with Asimov).
   - c.f. Issue #1911
* Add tests and test XML files.

@kratsg kratsg added the fix A bug fix label Aug 10, 2022
@kratsg kratsg self-assigned this Aug 10, 2022
@codecov
Copy link

codecov bot commented Aug 10, 2022

Codecov Report

Merging #1931 (f150460) into master (1494b66) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1931      +/-   ##
==========================================
+ Coverage   98.17%   98.22%   +0.04%     
==========================================
  Files          68       68              
  Lines        4330     4330              
  Branches      729      729              
==========================================
+ Hits         4251     4253       +2     
+ Misses         46       45       -1     
+ Partials       33       32       -1     
Flag Coverage Δ
contrib 26.37% <0.00%> (ø)
doctest 60.66% <0.00%> (ø)
unittests-3.10 96.09% <100.00%> (+0.04%) ⬆️
unittests-3.7 96.08% <100.00%> (+0.04%) ⬆️

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

Impacted Files Coverage Δ
src/pyhf/readxml.py 95.78% <100.00%> (+1.20%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@matthewfeickert matthewfeickert added the tests pytest label Aug 10, 2022
Copy link
Member

@matthewfeickert matthewfeickert left a comment

Choose a reason for hiding this comment

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

LGTM. 👍 Thanks @kratsg!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix A bug fix tests pytest

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants