Skip to content
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

Add unit tests for qt_utils #112

Merged
merged 7 commits into from
Jun 22, 2021
Merged

Conversation

alex-simm
Copy link
Collaborator

@alex-simm alex-simm commented Jun 15, 2021

What

Add tests for utils/qt_utils.py

Why

Part of #92

How

Adds unit tests for some utility functions in qt_utils, which were not covered by tests yet.

Notes

I tried to test the properties of the function's results as described in #71, so that the tests are independent of the function's implementation. For functions which can work with arbitrary dimensions (e.g. xy_basis), the test is repeated for some dimensions to be sure, e.g dim=3, 5, 10, 100. Going through all dimensions up to some max value would probably make the test run unnecessary long.

@CLAassistant
Copy link

CLAassistant commented Jun 15, 2021

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Jun 15, 2021

Codecov Report

Merging #112 (2130924) into dev (4321c2a) will increase coverage by 0.86%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #112      +/-   ##
==========================================
+ Coverage   57.57%   58.44%   +0.86%     
==========================================
  Files          36       36              
  Lines        5304     5304              
==========================================
+ Hits         3054     3100      +46     
+ Misses       2250     2204      -46     
Impacted Files Coverage Δ
c3/utils/qt_utils.py 76.96% <0.00%> (+22.54%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4321c2a...2130924. Read the comment docs.

@pull-request-size pull-request-size bot added size/L and removed size/M labels Jun 16, 2021
@alex-simm alex-simm marked this pull request as ready for review June 18, 2021 07:48
@lazyoracle
Copy link
Member

Can you rebase on dev and force-push again?

Assuming you have the local repo set up as per the contribution guidelines:

git checkout dev
git fetch upstream dev
git checkout test-qt_utils
git rebase upstream/dev
git push -f origin test-qt_utils

@lazyoracle lazyoracle added the code-quality General code quality related issues and PRs label Jun 18, 2021
@alex-simm
Copy link
Collaborator Author

Can you rebase on dev and force-push again?

Assuming you have the local repo set up as per the contribution guidelines:

git checkout dev
git fetch upstream dev
git checkout test-qt_utils
git rebase upstream/dev
git push -f origin test-qt_utils

@alex-simm alex-simm closed this Jun 18, 2021
@alex-simm alex-simm reopened this Jun 18, 2021
test/test_qt_utils.py Outdated Show resolved Hide resolved
nwittler
nwittler previously approved these changes Jun 18, 2021
Copy link
Collaborator

@nwittler nwittler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and very thorough when @lazyoracle s minor comment is addressed.

test/test_qt_utils.py Outdated Show resolved Hide resolved
Copy link
Member

@lazyoracle lazyoracle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ready to merge.
Congratulations on the first PR! 🥳

@lazyoracle lazyoracle merged commit 31e96be into q-optimize:dev Jun 22, 2021
@alex-simm alex-simm deleted the test-qt_utils branch August 3, 2021 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-quality General code quality related issues and PRs size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants