Pytest Warnings

[purva-thakre]

Description

Fixes #196

Second attempt to fix this. Previous PR (#210) had ended up with commits from other branches in a git rebase.

Todos

Notable points that this PR has either accomplished or will accomplish.

• TODO 1

Questions

• Question1

Status

• Ready to go

[codecov]

Codecov Report

Merging #212 (e196a8f) into master (438e183) will decrease coverage by 0.1%.
The diff coverage is 100.0%.

@@           Coverage Diff            @@
##           master    #212     +/-   ##
========================================
- Coverage    96.0%   95.9%   -0.1%     
========================================
  Files         146     146             
  Lines        2954    2956      +2     
  Branches      728     728             
========================================
- Hits         2838    2837      -1     
- Misses         71      74      +3     
  Partials       45      45             

Files	Coverage Δ
toqito/matrices/gell_mann.py	100.0% <100.0%> (ø)
toqito/matrices/pauli.py	100.0% <ø> (ø)
toqito/matrix_ops/vectors_to_gram_matrix.py	100.0% <100.0%> (ø)
toqito/perms/symmetric_projection.py	100.0% <100.0%> (ø)
toqito/state_props/log_negativity.py	100.0% <100.0%> (ø)
toqito/state_props/negativity.py	100.0% <100.0%> (ø)

... and 1 file with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

[purva-thakre]

To do: Remove coverage completely.

coverage is still being installed even though an attempt to remove it was made in a524fcf

3.11 workflow raised a warning related to coverage.

https://github.com/vprusso/toqito/actions/runs/6799327748/job/18485402911#step:5:668

Edit: pytest-cov has coverage as a dependency. This package will be installed even though it is not listed in the toml file.

[purva-thakre]

@vprusso There are 3 warnings raised in the virtual environment by 2 third-party packages which are still unresolved.

Based on your previous comment, I am marking this PR ready for review.

[vprusso]

I left a few minor comments in the diff, but overall, I think it looks good!

I'll approve for now modulo some of those minor changes. Feel free to reach out and ask any questions or respond with any disagreements you may have about my comments!

[vprusso]

Hmm, I'm not sure if we want to ignore the CVXPY stuff. Should we keep these warnings in just in case they are actually useful?

[purva-thakre]

Most of the third party package deprecation warnings cannot be handled on our end. I feel it does clog up the pytest-cov output. The way I understand this, all the deprecations related to cvxpy dependencies will be ignored. But we should still be able to get the Deprecation warnings from cvxpy itself. I'll double check and link somethings I found later.

Edit: @vprusso you are right. I think this does ignore the warnings that we would prefer to not ignore. Will work on these.

tests/test_matrix_props/test_is_block_positive.py::test_swap_operator_is_block_positive[4]
tests/test_matrix_props/test_is_block_positive.py::test_is_block_positive
tests/test_state_props/test_has_symmetric_extension.py::test_has_symmetric_extension_level_2_entangled_false_ppt
  /home/runner/.cache/pypoetry/virtualenvs/toqito-QfgmswsN-py3.10/lib/python3.10/site-packages/cvxpy/problems/problem.py:164: UserWarning: Constraint #2 contains too many subexpressions. Consider vectorizing your CVXPY code to speed up compilation.
    warnings.warn(f"Constraint #{i} contains too many subexpressions. "

tests/test_matrix_props/test_is_block_positive.py::test_choi_is_block_positive
tests/test_matrix_props/test_is_block_positive.py::test_is_block_positive
tests/test_state_props/test_has_symmetric_extension.py::test_has_symmetric_extension_level_2_entangled_false_ppt
  /home/runner/.cache/pypoetry/virtualenvs/toqito-QfgmswsN-py3.10/lib/python3.10/site-packages/cvxpy/problems/problem.py:164: UserWarning: Constraint #4 contains too many subexpressions. Consider vectorizing your CVXPY code to speed up compilation.
    warnings.warn(f"Constraint #{i} contains too many subexpressions. "

tests/test_matrix_props/test_is_block_positive.py::test_is_block_positive
tests/test_state_props/test_has_symmetric_extension.py::test_has_symmetric_extension_level_2_entangled_false_ppt
  /home/runner/.cache/pypoetry/virtualenvs/toqito-QfgmswsN-py3.10/lib/python3.10/site-packages/cvxpy/problems/problem.py:164: UserWarning: Constraint #3 contains too many subexpressions. Consider vectorizing your CVXPY code to speed up compilation.
    warnings.warn(f"Constraint #{i} contains too many subexpressions. "

tests/test_state_props/test_has_symmetric_extension.py::test_has_symmetric_extension_level_2_entangled_false_ppt
  /home/runner/.cache/pypoetry/virtualenvs/toqito-QfgmswsN-py3.10/lib/python3.10/site-packages/cvxpy/problems/problem.py:164: UserWarning: Constraint #5 contains too many subexpressions. Consider vectorizing your CVXPY code to speed up compilation.
    warnings.warn(f"Constraint #{i} contains too many subexpressions. "

[vprusso]

Sounds good. For the "too many subexpressions" it's arguable whether we want to solve them in this diff or not. If it looks like doing that would be a rabbit hole, I'd say just keep the warnings as we might be able to address this later in another diff.

[purva-thakre]

I think the warning from cvxpy is not that helpful because they are not pointing to which exact function/constraint is problematic. It's a rabbit hole because the problematic line is not pointed out.

Edit: Decided to create a separate issue for this.

[purva-thakre]

Note : I have commented out a test in c2d8d89 because it was failing. This is the same test as in the description of another issue. #204 (comment)

I will tackle this problem after this PR's objective is completed (needed builds to pass). Changing package versions finally led to the failure that I was observing locally.

Adding an older comment for previously observed behavior:

the mastsumoto fidelity default test failure was happening because the value appears to fluctuate from the expected value of 0.99 to 0.89. If I run the test again a couple of times (or in a different environment), it will end up back to passing somehow. A quick glance at the code does not show any randomly chosen values.

I noticed the failure (with the calculated value being 0.89) happened in a new environment where I was checking if we needed to pin jsonschema. When I switched back to my old environment, all tests passed. I did make sure I was using the same branch in both envs. For these reasons, I do still think there is some dependency issue.

[vprusso]

Note : I have commented out a test in c2d8d89 because it was failing. This is the same test as in the description of another issue. #204 (comment)

I will tackle this problem after this PR's objective is completed (needed builds to pass). Changing package versions finally led to the failure that I was observing locally.

Adding an older comment for previously observed behavior:

the mastsumoto fidelity default test failure was happening because the value appears to fluctuate from the expected value of 0.99 to 0.89. If I run the test again a couple of times (or in a different environment), it will end up back to passing somehow. A quick glance at the code does not show any randomly chosen values.

I noticed the failure (with the calculated value being 0.89) happened in a new environment where I was checking if we needed to pin jsonschema. When I switched back to my old environment, all tests passed. I did make sure I was using the same branch in both envs. For these reasons, I do still think there is some dependency issue.

That is odd. Your approach sounds good though. Solving it here is outside of scope, but so long as it's tagged in another issue and dealt with there, that makes sense!

[purva-thakre]

Merging this.

Two unfinished changes have their own issues.

[vprusso]

Awesome. Great work, @purva-thakre !