-
Notifications
You must be signed in to change notification settings - Fork 8
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
SP3 clock normalisation & Helmert transformation #57
base: main
Are you sure you want to change the base?
Conversation
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.
This looks good to me. I've done a check for other functions that call gn_io.clk.sv_clocks
but I don't see anything else.
There is already a unit test for compare_clk()
so that's covered, but I cannot say anything about sp3_difference()
or clk_difference()
. If it's working as expected for your purposes then I think that's fine.
I would look at putting through unit tests for these two functions (doesn't have to be in this PR)
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.
As I said before, I think this is in a good enough state to go in, but I would wait on @seballgeyer to give his final approval before merging in
The one thing I would like to see is unit tests on functions that were touched here, but perhaps that could be done in a more wide-ranging PR that creates unit tests for a whole range of functions
Enable clock normalisation and Helmert transformation with sp3_difference