Skip to content

Conversation

@kratsg
Copy link
Contributor

@kratsg kratsg commented Aug 30, 2022

Pull Request Description

Resolves #1313.

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
* Use pdf.config.channels API instead of the channel names from the spec.
* Use pyhf.contrib.viz.brazil API for visualization.
* Deprecate use of pylab in notebook.

Co-authored-by: Matthew Feickert <[email protected]>

@kratsg kratsg added docs Documentation related fix A bug fix labels Aug 30, 2022
@kratsg kratsg self-assigned this Aug 30, 2022
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@codecov
Copy link

codecov bot commented Aug 30, 2022

Codecov Report

Base: 98.26% // Head: 98.26% // No change to project coverage 👍

Coverage data is based on head (cdcdf70) compared to base (cbd68c8).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1974   +/-   ##
=======================================
  Coverage   98.26%   98.26%           
=======================================
  Files          68       68           
  Lines        4440     4440           
  Branches      728      728           
=======================================
  Hits         4363     4363           
  Misses         45       45           
  Partials       32       32           
Flag Coverage Δ
contrib 27.22% <ø> (ø)
doctest 60.96% <ø> (ø)
unittests-3.10 96.19% <ø> (ø)
unittests-3.7 96.17% <ø> (ø)
unittests-3.8 96.21% <ø> (ø)
unittests-3.9 96.23% <ø> (ø)

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

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

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@alexander-held alexander-held left a comment

Choose a reason for hiding this comment

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

Looks good to me, I am fairly sure this fixed channel order is how this was always meant to be.

@matthewfeickert matthewfeickert force-pushed the docs/fixMultichannelNotebook branch from 4a4e0d9 to dd44831 Compare September 8, 2022 00:15
@matthewfeickert matthewfeickert changed the title fix: Multichannel Coupled Histo notebook bugs gave NaN fix: Form data through loop over pdf.config.channels for multichannel coupled histo notebook Sep 9, 2022
@matthewfeickert matthewfeickert changed the title fix: Form data through loop over pdf.config.channels for multichannel coupled histo notebook fix: Form data through loop over channels for multichannel coupled histo notebook Sep 9, 2022
@matthewfeickert matthewfeickert merged commit d48f7f2 into master Sep 9, 2022
@matthewfeickert matthewfeickert deleted the docs/fixMultichannelNotebook branch September 9, 2022 01:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation related fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

multichannel-coupled-histo notebook giving NAN with v0.6.0 API

3 participants