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.

Sign up for GitHub

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

Jump to bottom

Increase coverage for is_separable #509

Closed
wants to merge 14 commits into from
Closed

Increase coverage for is_separable #509

wants to merge 14 commits into from

Conversation

purva-thakre
Copy link
Collaborator

@purva-thakre purva-thakre commented Mar 13, 2024 •
edited
Loading

Description

Fixes #293

Changes

Requires tests for the following lines:

  • 74-79
  • 109-123
  • 131-179

Checklist

Before marking your PR ready for review, make sure you checked the following locally. If this is your first PR, you might be notified of some workflow failures after a maintainer has approved the workflow jobs to be run on your PR.

Additional information is available in the documentation.

  • Use ruff and pylint for errors related to code style and formatting.
  • Verify all previous and newly added unit tests pass in pytest.
  • Check the documentation build does not lead to any failures. Sphinx build can be checked locally for any failures related to your PR
  • Use linkcheck to check for broken links in the documentation
  • Use doctest to verify the examples in the function docstrings work as expected.

dependabot bot and others added 3 commits February 26, 2024 09:54
@dependabot
Update pylint requirement from 3.0.3 to 3.1.0
b092abb 
Updates the requirements on [pylint](https://github.com/pylint-dev/pylint) to permit the latest version.
- [Release notes](https://github.com/pylint-dev/pylint/releases)
- [Commits](pylint-dev/pylint@v3.0.3...v3.1.0)

---
updated-dependencies:
- dependency-name: pylint
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <[email protected]>
@purva-thakre
Merge branch 'master' into dependabot/pip/pylint-3.1.0
a60eaac
@purva-thakre
Merge branch 'master' of https://github.com/vprusso/toqito
e721fb5
@purva-thakre
Copy link
Collaborator Author

purva-thakre commented Mar 13, 2024 •
edited
Loading

Interestingly the following line shows up as uncovered in the coverage report even though there is a test covering the same lines in the unit tests.

toqito/toqito/state_props/is_separable.py

Lines 78 to 85 in b58f066

if isinstance(dim, int):
dim = np.array([dim, state_len / dim]) # pylint: disable=redefined-variable-type
if np.abs(dim[1] - np.round(dim[1])) >= 2 * state_len * eps:
raise ValueError("The parameter `dim` must evenly divide the length of the state.")
dim[1] = np.round(dim[1])
# nD, xD, pD
min_dim = int(min(dim))

toqito/toqito/state_props/tests/test_is_separable.py

Lines 23 to 28 in b58f066

def test_invalid_dim_parameter():
"""The dimension of the state must evenly divide the length of the state."""
with np.testing.assert_raises(ValueError):
dim = 3
rho = isotropic(dim, 1 / (dim + 1))
is_separable(rho, dim + 1)

Decided to remove these lines based on a comment in another PR: #335 (comment)

@purva-thakre
Copy link
Collaborator Author

purva-thakre commented Mar 13, 2024 •
edited
Loading

Another interesting thing is the changes in 57b25fc catches more lines as uncovered by unit tests.

Before

image

After
image

https://app.codecov.io/gh/vprusso/toqito/pull/509/blob/toqito/state_props/is_separable.py#L185

@codecov Codecov
Copy link

codecov bot commented Mar 13, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.1%. Comparing base (d1049fe) to head (5523138).

Additional details and impacted files 
@@           Coverage Diff            @@
##           master    #509     +/-   ##
========================================
- Coverage    98.1%   98.1%   -0.1%     
========================================
  Files         162     162             
  Lines        3112    3109      -3     
  Branches      760     759      -1     
========================================
- Hits         3054    3051      -3     
  Misses         37      37             
  Partials       21      21

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@purva-thakre purva-thakre mentioned this pull request Mar 13, 2024
Merged
6 tasks
@purva-thakre
Copy link
Collaborator Author

purva-thakre commented Mar 18, 2024 •
edited
Loading

I don't know why the following lines are showing up as uncovered in the pytes coverage.

toqito/toqito/state_props/is_separable.py

Lines 74 to 79 in a1fe746

if isinstance(dim, int):
dim = np.array([dim, state_len / dim]) # pylint: disable=redefined-variable-type
dim[1] = np.round(dim[1])
# nD, xD, pD
min_dim = int(min(dim))

The changes in a1fe746 should have covered these. Maybe, this is due to an error in Codecov? Can't see the full coverage report due to some error.

https://app.codecov.io/gh/vprusso/toqito/pull/509?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=checks&utm_campaign=pr+comments&utm_term=Vincent+Russo

Edit: does not appear to be a Codecov issue. I can see the report now but the linked lines are still uncovered.

@purva-thakre
Copy link
Collaborator Author

Temporarily closing this as the branch is out of date.

@purva-thakre purva-thakre deleted the issue_293 branch April 24, 2024 02:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add tests for toqito/state_props/is_separable.py: 77->84, 114->128, 136-188
1 participant
@purva-thakre