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
Fix 'allowed' configuration construct to deduce also later types than just the first one #27784
Conversation
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27784/11471
|
A new Pull Request was created by @makortel (Matti Kortelainen) for master. It involves the following packages: FWCore/ParameterSet @cmsbuild, @smuzaffar, @Dr15Jones can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild, please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
+1 |
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
While working with something else I came to wonder the indentation level of
in
_AllowedParameterTypes.__call__()
. It looked to me that the loop where theif
is would execute exactly one iteration over the allowed types. Extending the unit tests to check also setting of one of the later types confirmed that this is indeed the case. This PR proposes to fix the problem by moving the check above to be done after the loop over types (which possibly was the intention).In addition, I added tests to exercise the exception in ambiguous cases.
PR validation:
Framework unit tests run.