Xtrig arg validate #5955
Xtrig arg validate #5955
Conversation
…examples for each of the built in xtriggers. - Xrandom validate function - Init test xrandom validate function - Add unit tests for validation of built in xtriggers
|
@markgrahamdawson requesting a second review because this is quite collab between Hilary and I at this point. |
* Remove redundant try block. * Fix changelog * Automatically validate xtrigger function signature --------- Co-authored-by: Hilary James Oliver <hilary.j.oliver@gmail.com>
Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
Fixed locally. Reopened so CI runs. Hopefully works on CI too. |
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
While looking at the test failures, I realised the implementation of xrandom's validate function was flawed with respect whether you used args vs kwargs.
Then I realised I could simplify the way validation is carried out, PR open at wxtim#60
* Improve xtrigger validation * wall_clock: use placeholder function for signature validation & autodocs * Fix docstring for autodoc [skip ci]
|
Kicking tests |
|
Ah right, @wxtim when you squash merge a PR make sure you remove |
|
@wxtim Any objection to merging this? I think enough due process has occurred at this point, seeing as I have reviewed yours/Hilary's code and you've reviewed mine? |
That seems reasonable - All the code has been reviewed by two pairs of eyes in effect even if the review was not linear. |
Close #5448
Replaces #5452, to which I have added tests.
From @hjoliver
From @MetRonnie (added to this PR)
Check List
CONTRIBUTING.mdand added my name as a Code Contributor.setup.cfg(andconda-environment.ymlif present).CHANGES.mdentry included if this is a change that can affect users?.?.xbranch.