-
Notifications
You must be signed in to change notification settings - Fork 15
Increase codecov coverage to 80%, or higher #205
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
Conversation
|
Hi aarontrowbridge, does this PR bring the test coverage above 80%? And could I potentially start working on bringing it above 95% for another bounty? |
|
@jack-champagne I'm seeing a missing head commit at https://app.codecov.io/gh/harmoniqs/QuantumCollocation.jl/pull/205 when trying to check the status |
@andgoldschmidt is there anything I can do to fix the missing head commit? |
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
|
Ok we did fix the codecov issues, looks like we're at 79.57% 🙃 I guess it does round up! One of us will look closer and see if we have any feedback on this before we go ahead and accept. |
|
Oh wow :D Thanks for the round up! Let me know if any changes are necessary! |
|
@andgoldschmidt are we able to close this issue and issue #203 by tomorrow? Because 11.6. is the deadline for UnitaryHack bounties |
|
Thanks for the review @andgoldschmidt ! I have addressed all your comments in my last commit. One issue I encountered was the lift_operator function, I was not able to import it in any way, it doesn't seem to be in a module. So I copied it to transmons.jl as a temporary fix. I hope this would be a quick-fix for you and we'd be able to close this issue by midnight CDT :) |
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.
should be able to import lift_operator then good to go
|
the lift_opertor is used in the function TransmonDipoleCoupling, not a test. I added 'using PiccoloQuantumObjects' at the top of the file, but I get error with all of these options: |
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.
Leaving in the TODO from PiccoloQuantumObjects, this is good to go
This PR fixes issue #204
Added tests to files:
transmons.jl
trajectory_initialization.jl
Hopefully achieved test coverage above 80%.
Made necessary changes to make all tests work