Refactor the test suite using pytest for array and matrix types #438

bnavigator · 2020-07-25T22:04:22Z

Inspired by the discussion in #433 I created a pytest fixture matarrayout that parametrizes all the test functions and runs them both with use_numpy_matrix(True) and ...(False) for the two available output conventions. To turn on the autouse fixture, export the environment variable PYTHON_CONTROL_ARRAY_AND_MATRIX=1. There is an additional Travis CI job for this.

It turned out, that the unittest.TestCase inheriting test classes don't like using the autoused fixture. So one thing let to the other and I converted all the test files to conform to pytest.

Test suite changes

There are several tests that use config.reset_defaults() working against the functionality of the new fixture. So I changed that too and created the fixture editsdefaults. BTW, I think defaults is a misnomer. It should be settings that can be reset to default values.
robust_array_test.py is removed and its functionality completely replaced by the usage of matarrayin in robust_test.py
statefbk_array_test.py and state_fbk_matrix_test.py are merged into statefbk_test.py with matarrayin.
statesp_array_test.py is merged into statesp_test.py
timeresp_test.py looks completeley different now. The tests are organized more systematically to test the time response functions with a more complete set of parameter variations. This revealed the issue described in do not override squeeze parameter in forced_response #442.
xferfcn_test.py::test_matrix_multiply() now checks both left and right multiplication with np.matrix and is prepared for array matmul, if we ever succeed to implement it (currently xfails for array).
the Travis CI test matrix is extended to Python 3.8 and the matrix definition is cleaned up.
There are a few new named pytest marks importable from control.tests.conftest which can be used as decorators on tests for easier definition of skips:
- @slycotonly
- @nopython2
- @noscipy0
The matarrayout fixture updates the warnings filter for numpy matrix deprecation warnings. No global filter anymore, so unexpected usage gets revealed for matarrayout=np.array.
For checking that functions work with matrix or arrays as input, you can use the fixture matarrayin. The new statefbk_test.py makes use of this -- No test code should directly use deprecated np.matrix anymore.

coveralls · 2020-07-25T22:15:30Z

Coverage increased (+0.4%) to 87.04% when pulling 2b98769 on bnavigator:array-matrix-tests into 4eee1f3 on python-control:master.

bnavigator · 2020-07-25T22:17:48Z

control/tests/convert_test.py

+                ssxfrm_real = ssxfrm_mag * np.cos(ssxfrm_phase)
+                ssxfrm_imag = ssxfrm_mag * np.sin(ssxfrm_phase)
+                np.testing.assert_array_almost_equal(
+                    ssorig_real, ssxfrm_real, decimal=5)


I had to reduce precision here, because running the test with matrix and array advances the random seed and produces different random systems.

bnavigator · 2020-07-25T22:20:43Z

control/tests/discrete_test.py

-        try:
-            tf2*tf3             # Error; incompatible timebases
-            raise ValueError("incompatible operation allowed")
-        except ValueError:
-            pass
-        try:


The pytest code makes this look a lot cleaner.

bnavigator · 2020-07-25T22:22:26Z

control/tests/frd_test.py

-            # Make sure that we get a pending deprecation warning
-            self.assertRaises(PendingDeprecationWarning, frd_tf.evalfr, 1.)
+        with pytest.deprecated_call():
+            frd_tf.evalfr(1.)



Pytest takes care of this just fine

bnavigator · 2020-07-25T22:24:55Z

control/tests/robust_array_test.py

@@ -1,393 +0,0 @@
-import unittest


Same code as robust_test.py, just different setup and teardown. So obsolete with the new fixture.

bnavigator · 2020-07-26T00:39:10Z

the iosys_test now fails with incompatible timebases. probably need to merge #431 too. Has some merge conflicts, will need to rebase and apply #431, #433 in order...

bnavigator · 2020-07-26T14:52:34Z

@sawyerbfuller, can I pick 8e3ddd2 from your branch fix-lti-evalfr into this PR and pytestify it right away? That way you don't have to submit a PR yourself and I don't have to rebase those fixes later.

sawyerbfuller · 2020-07-26T16:00:07Z

Hi ben , I agree that pull request will need to happen after the changes you’ve been making. Right now though, that branch Has changes that I don’t think are the right way to do it (incorrectly assumes omega is real, and I think we should move toward using __call__) so please don’t merge. S

On Sun, Jul 26, 2020 at 7:53 AM Ben Greiner ***@***.***> wrote: @sawyerbfuller <https://github.com/sawyerbfuller>, can I pick 8e3ddd2 <8e3ddd2> from your branch fix-lti-evalfr into this PR and pytestify it right away? That way you don't have to submit a PR yourself and I don't have to rebase those fixes later. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#438 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AN74SSOBSVQL4M4UFQGXYLTR5Q7L7ANCNFSM4PHU6UNA> .

-- Sawyer Fuller Assistant Professor of Mechanical Engineering University of Washington http://faculty.washington.edu/~minster/ (Typed with my thumbs)

sawyerbfuller · 2020-07-27T17:53:47Z

BTW, I think 'defaults' is a misnomer. It should be 'settings' that can be reset to default values.

I agree with this statement.

…-control#438)

(backported from python-control#438)

remove a lot of duplicate code by converting everything into a single parametrized test function.

bnavigator · 2020-12-30T00:12:48Z

Rebase is done. I pulled everything not directly related to the testsuite out into different PRs. The new timevector calculation needs a minor change (#485) to fix the failing tests here.

revert this commit when merging a rebased python-control#431 (remove statesp_test.py::test_copy_constructor_nodt if not applicable)

murrayrm

A few small things that could be fixed, but OK to merge.

murrayrm · 2020-12-30T16:57:51Z

.travis.yml

+  - "3.8"
  - "3.7"


To minimize CI resource usage, would it make sense to just check against the earliest supported Python 3 release and the latest stable Python 3 release? So 3.6 and 3.9, with the idea that if it works in those two then 3.7, and 3.8 are probably OK as well.

Definitely. AFAIK we do not make use of any of the changed Python stdlib changes like importlib-metadata, -resources, -dataclasses or the like.

We will need to revisit the test strategy matrix when moving from travis-ci.org to travis-ci.com or Github Actions. (#446)

murrayrm · 2020-12-30T17:19:58Z

setup.cfg

@@ -4,5 +4,4 @@ universal=1
 [tool:pytest]
 filterwarnings =
    ignore:.*matrix subclass:PendingDeprecationWarning
-    ignore:.*scipy:DeprecationWarning


Do we really want to ignore these? Seems like something we would want to flag in a CI test.

This change removes one ignore. We can also remove the other and should track down any remaining matrix usage (in a separate PR, if outside of the test suite). Without the remaining filter, I get this summary:

============================================= warnings summary ============================================== control/tests/bdalg_test.py: 387 warnings control/tests/canonical_test.py: 180 warnings control/tests/config_test.py: 8 warnings control/tests/convert_test.py: 3080 warnings control/tests/discrete_test.py: 964 warnings control/tests/flatsys_test.py: 54 warnings control/tests/frd_test.py: 87 warnings control/tests/freqresp_test.py: 84 warnings control/tests/input_element_int_test.py: 8 warnings control/tests/iosys_test.py: 728 warnings control/tests/lti_test.py: 8 warnings control/tests/margin_test.py: 20 warnings control/tests/mateqn_test.py: 30 warnings control/tests/matlab2_test.py: 96 warnings control/tests/matlab_test.py: 896 warnings control/tests/minreal_test.py: 4496 warnings control/tests/modelsimp_test.py: 36 warnings control/tests/statefbk_test.py: 53 warnings /home/greiner/src/python-control/control/statesp.py:102: PendingDeprecationWarning: the matrix subclass is not the recommended way to represent matrices or deal with linear algebra (see https://docs.scipy.org/doc/numpy/user/numpy-for-matlab-users.html). Please adjust your code to use regular ndarray. arr = np.matrix(data, dtype=float) control/tests/iosys_test.py::TestIOSys::test_named_signals_linearize_inconsistent control/tests/iosys_test.py::TestIOSys::test_named_signals_linearize_inconsistent control/tests/iosys_test.py::TestIOSys::test_named_signals_linearize_inconsistent control/tests/iosys_test.py::TestIOSys::test_named_signals_linearize_inconsistent control/tests/matlab_test.py::TestMatlab::testDcgain control/tests/matlab_test.py::TestMatlab::testDcgain control/tests/statefbk_test.py::TestStatefbk::test_LQE[arrayin] control/tests/statefbk_test.py::TestStatefbk::test_LQE[arrayin] /usr/lib64/python3.8/site-packages/numpy/matrixlib/defmatrix.py:69: PendingDeprecationWarning: the matrix subclass is not the recommended way to represent matrices or deal with linear algebra (see https://docs.scipy.org/doc/numpy/user/numpy-for-matlab-users.html). Please adjust your code to use regular ndarray. return matrix(data, dtype=dtype, copy=False) -- Docs: https://docs.pytest.org/en/stable/warnings.html

These warnings vanish, when I activate PYTHON_CONTROL_ARRAY_AND_MATRIX=1. Seems like the default when this is not set is wrong. Will track it down in a few hours.

murrayrm · 2020-12-30T17:20:58Z

setup.py

 Programming Language :: Python :: 3.6
 Programming Language :: Python :: 3.7
 Programming Language :: Python :: 3.8


Not sure whether we should only list versions we are explicitly testing against (so this would match .travis.yml list).

…" from python-control#438 This reverts commit 2b98769.

bnavigator commented Jul 25, 2020

View reviewed changes

bnavigator force-pushed the array-matrix-tests branch 2 times, most recently from 9500543 to b7b7f7b Compare July 26, 2020 00:05

bnavigator force-pushed the array-matrix-tests branch 3 times, most recently from 273a80b to e7affff Compare July 26, 2020 14:31

bnavigator force-pushed the array-matrix-tests branch from 9804e7b to 8ee73b9 Compare July 26, 2020 21:42

bnavigator mentioned this pull request Jul 29, 2020

Return type of eigenvalues from lqr and lqe not consistent #439

Closed

bnavigator force-pushed the array-matrix-tests branch from 0bd120d to 45d677f Compare July 29, 2020 21:12

This was referenced Jul 29, 2020

Complex matrices #376

Closed

do not override squeeze parameter in forced_response #442

Merged

bnavigator force-pushed the array-matrix-tests branch 4 times, most recently from 90582e5 to da1c517 Compare August 1, 2020 13:46

This was referenced Aug 1, 2020

Fix matlab.stepinfo() #444

Merged

Fixed InterconnectedSystems name bugs. #400

Merged

bnavigator marked this pull request as ready for review August 1, 2020 18:21

bnavigator added a commit to bnavigator/python-control that referenced this pull request Aug 2, 2020

cover TransferFunction.returnScipySignalLTI() (backported from python…

ba7817c

…-control#438)

bnavigator added a commit to bnavigator/python-control that referenced this pull request Aug 2, 2020

returnScipySignalLTI for discrete systems and add unit tests

a8aa41e

(backported from python-control#438)

bnavigator force-pushed the array-matrix-tests branch 2 times, most recently from e96f365 to ab77e02 Compare August 2, 2020 22:55

bnavigator added 18 commits December 30, 2020 00:56

pytestify input_element_int_test

caed712

pytestify iosys_test

55a6119

pytestify lti_test.py

521e927

pytestify mateqn_test

2c31029

pytestify matlab tests

fc2f770

pytestify minreal_test

5880473

pytestify modelsimp

97eedaf

pytestify nichols_test

3eb0a92

pytestify phaseplot_test

000998f

pytestify rlocus_test.py

3177796

pytestify robust tests

f745bd2

pytestify sisotool_test.py

2ee0773

pytestify slycot_convert_test.py

e660363

pytestify statefbk tests

952c511

pytestify statesp tests

6386832

pytestify timeresp_test.py

1dd2525

pytestify xferfcn_input_test.py

29883e8

remove a lot of duplicate code by converting everything into a single parametrized test function.

pytestify xferfcn tests

ca9476f

bnavigator force-pushed the array-matrix-tests branch from 7674d30 to deeb0c7 Compare December 29, 2020 23:56

bnavigator mentioned this pull request Dec 30, 2020

deprecate np.matrix usage #486

Merged

make tests work with pre python-control#431 source code state

2b98769

revert this commit when merging a rebased python-control#431 (remove statesp_test.py::test_copy_constructor_nodt if not applicable)

bnavigator force-pushed the array-matrix-tests branch from deeb0c7 to 2b98769 Compare December 30, 2020 16:54

murrayrm reviewed Dec 30, 2020

View reviewed changes

murrayrm merged commit c432fd5 into python-control:master Dec 30, 2020

murrayrm mentioned this pull request Dec 30, 2020

reduce Python 3 testing to speed up Travis CI testing #487

Merged

bnavigator mentioned this pull request Dec 30, 2020

Unit tests do still not use array but matrix by default #488

Closed

bnavigator added a commit to bnavigator/python-control that referenced this pull request Dec 31, 2020

Revert "make tests work with pre python-control#431 source code state…

578cfa6

…" from python-control#438 This reverts commit 2b98769.

bnavigator mentioned this pull request Dec 31, 2020

Rebased #431: Default dt is 0 #490

Merged

bnavigator deleted the array-matrix-tests branch February 18, 2024 20:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor the test suite using pytest for array and matrix types #438

Refactor the test suite using pytest for array and matrix types #438

bnavigator commented Jul 25, 2020 •

edited

Loading

coveralls commented Jul 25, 2020 •

edited

Loading

bnavigator Jul 25, 2020

bnavigator Jul 25, 2020

bnavigator Jul 25, 2020

bnavigator Jul 25, 2020

bnavigator commented Jul 26, 2020

bnavigator commented Jul 26, 2020

sawyerbfuller commented Jul 26, 2020 via email

sawyerbfuller commented Jul 27, 2020

bnavigator commented Dec 30, 2020

murrayrm left a comment

murrayrm Dec 30, 2020

bnavigator Dec 30, 2020

murrayrm Dec 30, 2020

bnavigator Dec 30, 2020

murrayrm Dec 30, 2020

Refactor the test suite using pytest for array and matrix types #438

Refactor the test suite using pytest for array and matrix types #438

Conversation

bnavigator commented Jul 25, 2020 • edited Loading

Test suite changes

coveralls commented Jul 25, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnavigator commented Jul 26, 2020

bnavigator commented Jul 26, 2020

sawyerbfuller commented Jul 26, 2020 via email

sawyerbfuller commented Jul 27, 2020

bnavigator commented Dec 30, 2020

murrayrm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnavigator commented Jul 25, 2020 •

edited

Loading

coveralls commented Jul 25, 2020 •

edited

Loading