Skip to content

Comments

Test rewrite#266

Merged
sfarrens merged 34 commits intoCEA-COSMIC:developfrom
paquiteau:test_rewrite
Jan 3, 2023
Merged

Test rewrite#266
sfarrens merged 34 commits intoCEA-COSMIC:developfrom
paquiteau:test_rewrite

Conversation

@paquiteau
Copy link
Contributor

@paquiteau paquiteau commented Dec 7, 2022

This pull request rewrite all the test of Modopt to move to pytest.

  • Coverage is nearly identical
  • tests are significantly shorter, with little overhead complexity
  • I introduced the use of pytest-cases to easily apply the same test to different scenario
  • Using pytest-xdist and no unittest flavored tests, the test suite runs significantly faster!
  • The test files are black formatted, anticipating the changes we discussed

In the coming days I plan to improve coverage of the new test suite .

old coverage:

| Name                                      | Stmts | Miss | Cover | Missing                                                                                      |
|-------------------------------------------+-------+------+-------+----------------------------------------------------------------------------------------------|
| modopt/__init__.py                        |     7 |    0 |  100% |                                                                                              |
| modopt/base/__init__.py                   |     2 |    0 |  100% |                                                                                              |
| modopt/base/backend.py                    |    64 |   40 |   38% | 19-25, 35-39, 42-46, 68-74, 96-97, 99-100, 128, 152-160, 185-197, 221-231                    |
| modopt/base/np_adjust.py                  |    26 |    0 |  100% |                                                                                              |
| modopt/base/observable.py                 |    82 |   56 |   32% | 62-63, 78-79, 98-114, 127, 147-149, 162-163, 176-177, 219-229, 243-253, 261-270, 284-289     |
| modopt/base/transform.py                  |    38 |    0 |  100% |                                                                                              |
| modopt/base/types.py                      |    38 |    0 |  100% |                                                                                              |
| modopt/base/wrappers.py                   |    13 |    0 |  100% |                                                                                              |
| modopt/math/__init__.py                   |     2 |    0 |  100% |                                                                                              |
| modopt/math/convolve.py                   |    23 |    0 |  100% |                                                                                              |
| modopt/math/matrix.py                     |    74 |    0 |  100% |                                                                                              |
| modopt/math/metrics.py                    |    57 |    9 |   84% | 20, 80-83, 127-134                                                                           |
| modopt/math/stats.py                      |    31 |    0 |  100% |                                                                                              |
| modopt/opt/__init__.py                    |     2 |    0 |  100% |                                                                                              |
| modopt/opt/algorithms/__init__.py         |     5 |    0 |  100% |                                                                                              |
| modopt/opt/algorithms/base.py             |    82 |   14 |   83% | 94-101, 120, 252-253, 277-284, 288, 315-318, 321, 334                                        |
| modopt/opt/algorithms/forward_backward.py |   277 |   19 |   93% | 89, 103-104, 159, 168, 173, 216, 386, 502, 517, 619, 786, 801, 921, 979-980, 984, 1032, 1052 |
| modopt/opt/algorithms/gradient_descent.py |   118 |    7 |   94% | 107, 157, 210, 226, 311, 408, 410                                                            |
| modopt/opt/algorithms/primal_dual.py      |    73 |    3 |   96% | 249, 270, 281                                                                                |
| modopt/opt/cost.py                        |    58 |    0 |  100% |                                                                                              |
| modopt/opt/gradient.py                    |    70 |    0 |  100% |                                                                                              |
| modopt/opt/linear.py                      |    76 |    5 |   93% | 92-99                                                                                        |
| modopt/opt/proximity.py                   |   314 |   24 |   92% | 20-21, 338, 624, 935, 1013, 1073-1074, 1084-1118, 1135, 1147, 1299                           |
| modopt/opt/reweight.py                    |    19 |    1 |   95% | 84                                                                                           |
| modopt/signal/__init__.py                 |     2 |    0 |  100% |                                                                                              |
| modopt/signal/filter.py                   |    20 |    0 |  100% |                                                                                              |
| modopt/signal/noise.py                    |    29 |    0 |  100% |                                                                                              |
| modopt/signal/positivity.py               |    18 |    1 |   94% | 108                                                                                          |
| modopt/signal/svd.py                      |    59 |    1 |   98% | 240                                                                                          |
| modopt/signal/validation.py               |    13 |    0 |  100% |                                                                                              |
| modopt/signal/wavelet.py                  |    30 |   10 |   67% | 60-68, 197-201                                                                               |
|-------------------------------------------+-------+------+-------+----------------------------------------------------------------------------------------------|
TOTAL                                        2496    205    92%
Coverage XML written to file coverage.xml

NEW coverage:

| Name                                      | Stmts | Miss | Cover | Missing                                                                                                              |
|-------------------------------------------+-------+------+-------+----------------------------------------------------------------------------------------------------------------------|
| modopt/__init__.py                        |     7 |    0 |  100% |                                                                                                                      |
| modopt/base/__init__.py                   |     2 |    0 |  100% |                                                                                                                      |
| modopt/base/backend.py                    |    64 |   40 |   38% | 19-25, 35-39, 42-46, 68-74, 96-97, 99-100, 128, 152-160, 185-197, 221-231                                            |
| modopt/base/np_adjust.py                  |    26 |    0 |  100% |                                                                                                                      |
| modopt/base/observable.py                 |    82 |   56 |   32% | 62-63, 78-79, 98-114, 127, 147-149, 162-163, 176-177, 219-229, 243-253, 261-270, 284-289                             |
| modopt/base/transform.py                  |    38 |    1 |   97% | 56                                                                                                                   |
| modopt/base/types.py                      |    38 |   10 |   74% | 47, 141-153                                                                                                          |
| modopt/base/wrappers.py                   |    13 |    0 |  100% |                                                                                                                      |
| modopt/math/__init__.py                   |     2 |    0 |  100% |                                                                                                                      |
| modopt/math/convolve.py                   |    23 |    0 |  100% |                                                                                                                      |
| modopt/math/matrix.py                     |    74 |    0 |  100% |                                                                                                                      |
| modopt/math/metrics.py                    |    57 |    9 |   84% | 20, 80-83, 127-134                                                                                                   |
| modopt/math/stats.py                      |    31 |    0 |  100% |                                                                                                                      |
| modopt/opt/__init__.py                    |     2 |    0 |  100% |                                                                                                                      |
| modopt/opt/algorithms/__init__.py         |     5 |    0 |  100% |                                                                                                                      |
| modopt/opt/algorithms/base.py             |    82 |   17 |   79% | 94-101, 120, 177, 198, 222, 252-253, 277-284, 288, 315-318, 321, 334                                                 |
| modopt/opt/algorithms/forward_backward.py |   277 |   24 |   91% | 89, 103-104, 159, 168, 173, 216, 386, 502, 517, 619, 675-676, 681, 684, 692, 786, 801, 921, 979-980, 984, 1032, 1052 |
| modopt/opt/algorithms/gradient_descent.py |   118 |    7 |   94% | 107, 157, 210, 226, 311, 408, 410                                                                                    |
| modopt/opt/algorithms/primal_dual.py      |    73 |    4 |   95% | 168, 249, 270, 281                                                                                                   |
| modopt/opt/cost.py                        |    58 |    0 |  100% |                                                                                                                      |
| modopt/opt/gradient.py                    |    70 |    1 |   99% | 172                                                                                                                  |
| modopt/opt/linear.py                      |    76 |    6 |   92% | 92-99, 314                                                                                                           |
| modopt/opt/proximity.py                   |   314 |   35 |   89% | 20-21, 471-473, 519-522, 543-551, 572, 935, 1013, 1073-1074, 1084-1118, 1135, 1147, 1218-1222, 1299                  |
| modopt/opt/reweight.py                    |    19 |    1 |   95% | 84                                                                                                                   |
| modopt/signal/__init__.py                 |     2 |    0 |  100% |                                                                                                                      |
| modopt/signal/filter.py                   |    20 |    0 |  100% |                                                                                                                      |
| modopt/signal/noise.py                    |    29 |    1 |   97% | 80                                                                                                                   |
| modopt/signal/positivity.py               |    18 |    1 |   94% | 108                                                                                                                  |
| modopt/signal/svd.py                      |    59 |    1 |   98% | 240                                                                                                                  |
| modopt/signal/validation.py               |    13 |    1 |   92% | 57                                                                                                                   |
| modopt/signal/wavelet.py                  |    30 |   10 |   67% | 60-68, 197-201                                                                                                       |
|-------------------------------------------+-------+------+-------+----------------------------------------------------------------------------------------------------------------------|
TOTAL                                        2219    240    89%
Coverage XML written to file coverage.xml

@paquiteau
Copy link
Contributor Author

paquiteau commented Dec 8, 2022

@philouc I took your review into account.

For the testing parameters of the algorithm, I agree that the current setup is too simple-minded. I decided to stick to the existing implementation for now.
Better test scenario could be investigated (at a greater computational cost), and going further, we could even investigate an automated benchmark (along the lines of this for example). There might even be some synergies to look for with benchopt, but again that remains a far-fetched future.

@sfarrens
Copy link
Contributor

sfarrens commented Jan 2, 2023

Hi @paquiteau here is my first round of comments on your PR. This is really nice work and it will certainly make maintaining the unit tests easier going forward. I have not reviewed every single module yet as some questions I have raised will probably be the same for the other modules, so I will wait for your responses before I review the rest.

@paquiteau
Copy link
Contributor Author

Alright I took your review into account for the signal module.
Concerning the use of class to separate tests in a same file: it does not change anything to pytest discovery, and is unecessary in most cases. I used it sporadically when a single test file is use for a multiple submodules (like test_signals.py, and its consistent there now).
For other test files I use the classes to regroup cases function (from pytest-cases plugin), but test are layed flat in file, along with their associated fixtures (see for instance test_opt.py)

@paquiteau paquiteau requested review from philouc and sfarrens January 2, 2023 18:07
@sfarrens
Copy link
Contributor

sfarrens commented Jan 3, 2023

Hi @paquiteau I have made another pass and opened a couple of threads. I think once these points are fixed and the tests are passing we can merge this PR.

@paquiteau paquiteau requested a review from sfarrens January 3, 2023 13:07
@sfarrens
Copy link
Contributor

sfarrens commented Jan 3, 2023

I have also been getting this same error locally:

/usr/share/miniconda/envs/test/lib/python3.10/site-packages/pytest_pydocstyle.py:50: in pytest_collect_file
    for filename, _, _ in parser.get_files_to_check():
E   ValueError: too many values to unpack (expected 3)

I assume there is an issue with the latest version of pytest-pydocstyle. We can probably comment this option out in the setup.cfg and open a separate issue to investigate.

@paquiteau
Copy link
Contributor Author

After a quick investigation, the culprit is pydocstyle itself (there was 2 new release this morning, locally I was using pydocstyle 6.1.1, and the CI uses 6.2.1).
A quick fix could also be to pin the version of pydocstyle.

@sfarrens
Copy link
Contributor

sfarrens commented Jan 3, 2023

After a quick investigation, the culprit is pydocstyle itself (there was 2 new release this morning, locally I was using pydocstyle 6.1.1, and the CI uses 6.2.1). A quick fix could also be to pin the version of pydocstyle.

Yep, let's do that. We can pin to v6.1.1 until this is fixed. Note you will get a bunch of D107: Missing docstring in __init__ errors. So, if you don't wan't to deal with that in this PR, you can update setup.cfg with

[pydocstyle]
convention = numpy
add-ignore = D107

@sfarrens
Copy link
Contributor

sfarrens commented Jan 3, 2023

The only test not currently passing is related to #272, so I will go ahead and merge this PR. Nice work @paquiteau!

@sfarrens sfarrens merged commit eeb8d17 into CEA-COSMIC:develop Jan 3, 2023
@paquiteau paquiteau deleted the test_rewrite branch January 3, 2023 14:15
chaithyagr added a commit that referenced this pull request Jan 19, 2024
* Add support for tensorflow backend which allows for differentiability (#112)

* Added support for tensorflow

* Updates to get tests passing

* Or --> And

* Moving modopt to allow working with tensorflow

* Fix issues with wos

* Fix all flakes finally!

* Update modopt/base/backend.py

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update modopt/base/backend.py

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Minute updates to codes

* Add dynamic module

* Fix docu

* Fix PEP

Co-authored-by: chaithyagr <chaithyagr@gitlab.com>
Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Fix 115 (#116)

* Fix issues

* Add right tests

* Fix PEP

Co-authored-by: chaithyagr <chaithyagr@gitlab.com>

* Minor bug fix, remove elif (#124)

Co-authored-by: chaithyagr <chaithyagr@gitlab.com>

* Add tests for modopt.base.backend and fix minute bug uncovered (#126)

* Minor bug fix, remove elif

* Add tests for backend

* Fix tests

* Add tests

* Remove cupy

* PEP fixes

* Fix PEP

* Fix PEP and update

* Final PEP

* Update setup.cfg

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update test_base.py

Co-authored-by: chaithyagr <chaithyagr@gitlab.com>
Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Release cleanup (#128)

* updated GPU dependencies

* added logo to manifest

* updated package version and release date

* Unpin package dependencies (#189)

* unpinned dependencies

* updated pinned documentation dependency versions

* Add Gradient descent algorithms (#196)

* Version 1.5.1 patch release (#114)

* Add support for tensorflow backend which allows for differentiability (#112)

* Added support for tensorflow

* Updates to get tests passing

* Or --> And

* Moving modopt to allow working with tensorflow

* Fix issues with wos

* Fix all flakes finally!

* Update modopt/base/backend.py

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update modopt/base/backend.py

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Minute updates to codes

* Add dynamic module

* Fix docu

* Fix PEP

Co-authored-by: chaithyagr <chaithyagr@gitlab.com>
Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Fix 115 (#116)

* Fix issues

* Add right tests

* Fix PEP

Co-authored-by: chaithyagr <chaithyagr@gitlab.com>

* Minor bug fix, remove elif (#124)

Co-authored-by: chaithyagr <chaithyagr@gitlab.com>

* Add tests for modopt.base.backend and fix minute bug uncovered (#126)

* Minor bug fix, remove elif

* Add tests for backend

* Fix tests

* Add tests

* Remove cupy

* PEP fixes

* Fix PEP

* Fix PEP and update

* Final PEP

* Update setup.cfg

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update test_base.py

Co-authored-by: chaithyagr <chaithyagr@gitlab.com>
Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Release cleanup (#128)

* updated GPU dependencies

* added logo to manifest

* updated package version and release date

Co-authored-by: Chaithya G R <chaithyagr@gmail.com>
Co-authored-by: chaithyagr <chaithyagr@gitlab.com>

* make algorithms a module.

* add Gradient Descent Algorithms

* enforce WPS compliance.

* add test for gradient descent

* Docstrings improvements

* Add See Also and minor corrections

* add idx initialisation for all algorithms.

* fix merge error

* fix typo

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>
Co-authored-by: Chaithya G R <chaithyagr@gmail.com>
Co-authored-by: chaithyagr <chaithyagr@gitlab.com>

* Release cleanup (#198)

* started clean up for next release

* update progress

* further clean up

* additional clean up

* cleaned up link to logo

* fixed index.rst

* fixed conflict

* Fast Singular Value Thresholding  (#209)

* add SingularValueThreshold

This Method provides 10x faster SVT estimation than the LowRankMatrix Operator.

* linting

* add test for fast computation.

* flake8 compliance

* Ignore DAR000 Error.

* Update modopt/signal/svd.py

tuples in docstring

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update modopt/signal/svd.py

typo

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update modopt/opt/proximity.py

typo

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* update docstring

* fix isort

* Update modopt/signal/svd.py

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update modopt/signal/svd.py

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* run isort

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* added writeable input data array feature for benchopt (#213)

* removed flake8 limit

* updated patch version

* [lint] pydocstyle compliance. (#228)

* [lint] pydocstyle compliance.

* use pytest-pydocstyle

* Power method: fix #211 (#212)

* Correct the norm update for Power Method

x_new should be divided by its norm, not by x_old_norm.

* fix test value

We are testing for eigen value of Identity. It should be one.

* fix WPS350

* fix test value for unconverged case

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Switch from progressbar to tqdm (#231)

* switch from progressbar to tqdm.

The progress bar can be provided externally for nested usage.

* exposes the progress bar argument.

* Child classes better have to implement these.

(my linter was complaining)

* update docs for progress bar using tqdm.

* fix WPS errors

* drop progressbar requirement, add tqdm.

* [lint] disable warning for non implemented function.

* simplify progbar check and argument passthrough

* Update README for tqdm dependency (#240)

Remote progressbar, use tqdm.

* add small help for the metric argument. (#241)

* add small help for the metric argument.

* RST validation

* use single quote

* use double backticks.

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* add implementation for admm and fast admm.

Based on Goldstein2014

* add Goldstein ref.

* WPS compliance.

* Abstract class for cost function.

* add custom cost operator for admm.

* fix WPS compliance.

* Ci update (#268)

* update python version support.

* use string for CI.

* remove flake8 and wemake-python-styleguide

This anticipates the change to black formatting.

* remove wps checks

* apparently conda does not support 3.11 for now

* remove all linting testing.

* fix np.int warning/error

* fix dtype error

* fix precision for doctest

* added black and isort support

* Update python version in README

* add 3.7 for test  back

* don't test 3.10 twice

* Test rewrite (#266)

* add MatrixOperator.

* move base test to pytest.

* [fixme] remove flake8 and emoji config.

* rewrite test_math module using pytest.

* use fail/skipparam helper function.

* generalize usage of failparam

* refactor test_signal.

* refactor test_signal, the end.

* lint

* fix missing parameter.

* add dummy object test helper.

* rewrite test for cost and gradients.

* show missing lines in coverage reports

* rewrite of proximity operators testing.

* add fail  low rank method.

* add cases for algorithms test

* add algorithm test.

* add pytest-cases and pytest-xdists support.

* add support for testing metrics.

* improve base module coverage.

* test for wrong mask in metric module.

* add docstring.

* update email adress and authors field.

* 100% coverage for transform module.

* move linear operator to class

* update docstring.

* paramet(e)rization.

* update docstring.

* improve test_helper module.

* raises should be specified for each failparam call.

* encapsulate module's test in classes.

* skip test if sklearn is not installed.

* pin pydocstyle

* removed unnormalised Gaussian kernel option and corresponding test

* Restrict scikit-image version for testing

* added fix for basic test suite

* set behaviour for different astropy versions

* updated docstring for gaussian_kernel

* Use example scripts as  tests. (#277)

* Initialize the example module.

* do not export the assert statements.

* add matplotlib as requirement.

* add support for sphinx-gallery

* Update modopt/examples/README.rst

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update modopt/examples/__init__.py

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update modopt/examples/conftest.py

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update modopt/examples/example_lasso_forward_backward.py

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update modopt/examples/example_lasso_forward_backward.py

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* ignore auto_example folder

* doc formatting.

* add pogm and basic comparison.

* fix: add matplotlib for the plotting in examples scripts.

* fix: add matplotlib for basic ci too.

* ci: run pytest with xdist for faster testing

---------

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* fix: specify data_range for ssim.

Refs: #290

* typos.

* feat(test): add test for admm.

* feat(admm): improve doc.

* refactor: rename abstract cost to CostParent.

* feat: add test for fast admm.

* feat(admm): improve docstrings.

* style: remove extra line.c

* feat: make POGM more memory efficient.

* feat: add a dummy cost for the identity operator.

* feat: create a linear operator module, add wavelet transform.

* feat: add test case for wavelet transform.

* Update setup.py

---------

Co-authored-by: chaithyagr <chaithyagr@gitlab.com>
Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>
Co-authored-by: Pierre-Antoine Comby <77174042+paquiteau@users.noreply.github.com>
Co-authored-by: Pierre-antoine Comby <pierre-antoine.comby@crans.org>
Co-authored-by: Pierre-Antoine Comby <pierre-antoine.comby@ens-paris-saclay.fr>
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.

3 participants