-
Notifications
You must be signed in to change notification settings - Fork 85
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.
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
feat: Allow skipping of validation when combining workspaces #2385
Conversation
Is there a reason you want to combine workspaces that don't produce a valid workspace? |
Yes, I am working with workspaces that contain instructions for a custom modifier. These don't pass the validation in the first place, but for single workspaces I can simply skip it. Now I want to combine two of them, for a global fit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lorenzennio Can you also add your name to docs /contributors.rst
below
Line 35 in a1a31f1
- Jonas Rembser |
?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2385 +/- ##
=======================================
Coverage 98.28% 98.28%
=======================================
Files 69 69
Lines 4539 4539
Branches 803 803
=======================================
Hits 4461 4461
Misses 45 45
Partials 33 33
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your PR @lorenzennio — your contributions are appreciated! This all LGTM so I'm just going to push back some style changes that we don't have in the linter yet (nothing wrong with what you did, this is just to reduce noise later when we turn on some additional linting) and then I'll merge. 👍
Description
It would be nice to include the option to avoid the validation step when combining two workspaces:
pyhf/src/pyhf/workspace.py
Line 709 in a1a31f1
Checklist Before Requesting Reviewer
Before Merging
For the PR Assignees: