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 pytest for visualization.py #2192

Merged
merged 11 commits into from Jul 17, 2023

Conversation

tamakoshi2001
Copy link
Contributor

@tamakoshi2001 tamakoshi2001 commented Jul 9, 2023

Checklist
Thank you for contributing to QuTiP! Please make sure you have finished the following tasks before opening the PR.

  • Please read Contributing to QuTiP Development
  • Contributions to qutip should follow the pep8 style.
    You can use pycodestyle to check your code automatically
  • Please add tests to cover your changes if applicable.
  • If the behavior of the code has changed or new feature has been added, please also update the documentation in the doc folder, and the notebook. Feel free to ask if you are not sure.
  • Include the changelog in a file named: doc/changes/<PR number>.<type> 'type' can be one of the following: feature, bugfix, doc, removal, misc, or deprecation (see here for more information).

Delete this checklist after you have completed all the tasks. If you have not finished them all, you can also open a Draft Pull Request to let the others know this on-going work and keep this checklist in the PR description.

Description
pytest for #2170

@tamakoshi2001
Copy link
Contributor Author

Here is a sample pytest for functions in visualization.py

@coveralls
Copy link

coveralls commented Jul 9, 2023

Coverage Status

coverage: 83.199% (+5.0%) from 78.186% when pulling 385746c on tamakoshi2001:pytest-visualization into 7cf468c on qutip:master.

Comment on lines 6 to 27
@pytest.mark.parametrize('rho, x_basis, y_basis, color_style,\
label_top, cmap, colorbar, fig, ax', [
(qutip.rand_dm(5), None, None, "scaled", True, None, None, None, None),
(qutip.operator_to_vector(qutip.rand_dm(5)),
None, None, "scaled", True, None, None, None, None),
(qutip.operator_to_vector(qutip.rand_dm(5)).dag(),
None, None, "scaled", True, None, None, None, None),
(qutip.spre(qutip.rand_dm(4)),
None, None, "scaled", True, None, None, None, None),
(qutip.rand_dm(5),
[1, 2, 3, 4, 5], None, "scaled", True, None, None, None, None),
(qutip.rand_dm(5),
None, [1, 2, 3, 4, 5], "scaled", True, None, None, None, None),
(qutip.rand_dm(5), None, None, "scaled", True, None, None, None, None),
(qutip.rand_dm(5), None, None, "threshold", True, None, None, None, None),
(qutip.rand_dm(5), None, None, "phase", True, None, None, None, None),
])
def test_hinton(rho, x_basis, y_basis, color_style,
label_top, cmap, colorbar, fig, ax):
fig, ax = qutip.hinton(rho, x_basis, y_basis, color_style,
label_top, cmap=cmap, colorbar=colorbar,
fig=fig, ax=ax)
Copy link
Member

Choose a reason for hiding this comment

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

  • It's better not to call qutip function in the parametrization step, if there is a bug in that function, it stop all tests from running.
  • It's better to just parametrize on the variable we actually are interested in. All the None make it unclear what it tested in that particular test. It also makes test hard to read in the log:
qutip/tests/test_visualization.py::test_hinton[rho0-None-None-scaled-True-None-None-None-None] PASSED [ 16%]
qutip/tests/test_visualization.py::test_hinton[rho1-None-None-scaled-True-None-None-None-None] PASSED [ 16%]
qutip/tests/test_visualization.py::test_hinton[rho2-None-None-scaled-True-None-None-None-None] PASSED [ 16%]
qutip/tests/test_visualization.py::test_hinton[rho3-None-None-scaled-True-None-None-None-None] PASSED [ 16%]
qutip/tests/test_visualization.py::test_hinton[rho4-x_basis4-None-scaled-True-None-None-None-None] PASSED [ 16%]
qutip/tests/test_visualization.py::test_hinton[rho5-None-y_basis5-scaled-True-None-None-None-None] PASSED [ 16%]
qutip/tests/test_visualization.py::test_hinton[rho6-None-None-scaled-True-None-None-None-None] PASSED [ 16%]
qutip/tests/test_visualization.py::test_hinton[rho7-None-None-threshold-True-None-None-None-None] PASSED [ 16%]
qutip/tests/test_visualization.py::test_hinton[rho8-None-None-phase-True-None-None-None-None] PASSED [ 16%]
qutip/tests/test_visualization.py::test_hinton_ValueError[rho0-None-None-scaled-True-None-None-None-None-Input quantum object must be an operator or superoperator.] PASSED [ 16%]
qutip/tests/test_visualization.py::test_hinton_ValueError[rho1-None-None-color_style-True-None-None-None-None-Unknown color style color_style for Hinton diagrams.] PASSED [ 16%]
qutip/tests/test_visualization.py::test_hinton_ValueError[rho2-None-None-scaled-True-None-None-None-None-Hinton plots of superoperators are currently only supported for qubits.] PASSED [ 16%]
Suggested change
@pytest.mark.parametrize('rho, x_basis, y_basis, color_style,\
label_top, cmap, colorbar, fig, ax', [
(qutip.rand_dm(5), None, None, "scaled", True, None, None, None, None),
(qutip.operator_to_vector(qutip.rand_dm(5)),
None, None, "scaled", True, None, None, None, None),
(qutip.operator_to_vector(qutip.rand_dm(5)).dag(),
None, None, "scaled", True, None, None, None, None),
(qutip.spre(qutip.rand_dm(4)),
None, None, "scaled", True, None, None, None, None),
(qutip.rand_dm(5),
[1, 2, 3, 4, 5], None, "scaled", True, None, None, None, None),
(qutip.rand_dm(5),
None, [1, 2, 3, 4, 5], "scaled", True, None, None, None, None),
(qutip.rand_dm(5), None, None, "scaled", True, None, None, None, None),
(qutip.rand_dm(5), None, None, "threshold", True, None, None, None, None),
(qutip.rand_dm(5), None, None, "phase", True, None, None, None, None),
])
def test_hinton(rho, x_basis, y_basis, color_style,
label_top, cmap, colorbar, fig, ax):
fig, ax = qutip.hinton(rho, x_basis, y_basis, color_style,
label_top, cmap=cmap, colorbar=colorbar,
fig=fig, ax=ax)
def to_oper_bra(oper):
return qutip.operator_to_vector(oper).dag()
def to_oper(oper):
return oper
@pytest.mark.parametrize('transform, args', [
(to_oper, {}),
(qutip.operator_to_vector(), {}),
(to_oper_bra, {}),
(qutip.to_super(), {}),
(to_oper, {'x_basis': [1, 2, 3, 4, 5]),
(to_oper, {'y_basis': [1, 2, 3, 4, 5]),
(to_oper, {'color_style': 'threshold'),
(to_oper, {'color_style': 'phase'),
])
def test_hinton(transform, args):
rho = transform(qutip.rand_dm(5))
fig, ax = qutip.hinton(rho, **args)

@tamakoshi2001
Copy link
Contributor Author

@Ericgig Hi. The Github test fails because of plot_spin_distribution. plot_spin_distribution plots color at (x, y), but the correct way to use pcolor is plotting color in rectangles made by four matrix elements (the link). It may be difficult to fix this warning and hence we can not add pytests for it.
https://matplotlib.org/3.1.1/api/_as_gen/matplotlib.pyplot.pcolor.html

@Ericgig
Copy link
Member

Ericgig commented Jul 13, 2023

You can ignore warnings with:
@pytest.mark.filterwarnings("ignore:The input coordinates to pcolor:UserWarning")
The The input coordinates to pcolor is the start of the warning message.
UserWarning is the kind of warnings.

@tamakoshi2001
Copy link
Contributor Author

@Ericgig Hi. I added pytests for functions except for plot_wigner_sphere, matrix_histogram, and matrix_histogram_complex. Do you have any advice on them? I will cover inner functions this weekends

@Ericgig
Copy link
Member

Ericgig commented Jul 14, 2023

There are a few options:

Any is good, just tell me when you are ready for a review.

@tamakoshi2001
Copy link
Contributor Author

@Ericgig Hi. I prefer the latter. I am adding tests in #2193 now.

@tamakoshi2001
Copy link
Contributor Author

@Ericgig This PR is ready for review.

Copy link
Member

@Ericgig Ericgig left a comment

Choose a reason for hiding this comment

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

There are a few unneeded parametrization, but overall look great.

qutip/tests/test_visualization.py Outdated Show resolved Hide resolved
qutip/tests/test_visualization.py Show resolved Hide resolved
qutip/tests/test_visualization.py Outdated Show resolved Hide resolved
@Ericgig Ericgig merged commit 47ca07e into qutip:master Jul 17, 2023
12 checks passed
@tamakoshi2001 tamakoshi2001 changed the title pytest for visualization.py Add pytest for visualization.py Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants