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

tests: remove pytest from the mypy execution environment #87

Closed
wants to merge 4 commits into from

Conversation

muxator
Copy link

@muxator muxator commented Sep 13, 2024

Pytest is not needed for static type checking. Since tox.ini contains a duplicated list of dependencies (the authoritative one is in pyproject.toml), it is better to keep the one in tox.ini as short as possible.

muxator added 4 commits September 10, 2024 17:41
…ated and will be removed in a future release")

Before this change, the test log contained 37 entries like this:
    PytestRemovedIn8Warning: Support for nose tests is deprecated and will be removed in a future release.

After this change, on my machine the test log shrinks from 1672 to 1405 lines
(16% less).

At some point in time, we will migrate to Pytest 8.x, and this would be an
hindrance.

A reference for the fix can be found at:
    https://docs.pytest.org/en/stable/deprecations.html#support-for-tests-written-for-nose

Affected tests were found with:
    rg "def (setup|teardown)\(" tests
Before this change, the log of the tests were littered with messages from
Pillow:
    2024-09-10 18:04:55 [   DEBUG] STREAM b'IHDR' 16 13 (PngImagePlugin.py:197)
    2024-09-10 18:04:55 [   DEBUG] STREAM b'sBIT' 41 4 (PngImagePlugin.py:197)
    2024-09-10 18:04:55 [   DEBUG] b'sBIT' 41 4 (unknown) (PngImagePlugin.py:753)
    2024-09-10 18:04:55 [   DEBUG] STREAM b'pHYs' 57 9 (PngImagePlugin.py:197)

After this change, on my machine, the size of the log produced by running
"make test" goes from 1405 to 513 lines, a 63% reduction, that hopefully
increases the signal noise ratio of the test logs.
We have meaningful doctests in the code base. Before this change, their
execution had to be manually activated in Makefile and tox.ini.

This change centralizes the option in pytest.ini, so that we reduce the future
occurrence of human errors (less duplication, less oversights).
Pytest is not needed for static type checking. Since tox.ini contains a
duplicated list of dependencies (the authoritative one is in pyproject.toml), it
is better to keep the one in tox.ini as short as possible.
@muxator muxator marked this pull request as draft September 13, 2024 08:56
@muxator muxator force-pushed the remove-pytest-from-mypy-in-tox-ini branch from d5964ec to b04a7e8 Compare September 13, 2024 08:57
@muxator
Copy link
Author

muxator commented Sep 13, 2024

Withdrawing this PR: the tests themselves contain refererences to Pytest; mypy needs to have access to it (https://github.com/bancaditalia/black-it/actions/runs/10846166500/job/30098563227?pr=87).

$ rg --fixed-strings pytest. --files-with-matches 
tests/test_calibrator.py
tests/conftest.py
tests/test_search_space.py
tests/utils/base.py
tests/test_utils/test_sqlite3_checkpointing.py
tests/test_losses/test_base.py
tests/test_utils/test_base.py
tests/test_losses/test_msm.py
tests/test_samplers/test_gaussian_process.py
tests/test_samplers/test_best_batch.py
tests/test_samplers/test_particle_swarm.py
tests/test_examples/test_docker_sir.py
tests/test_examples/base.py
tests/test_examples/test_sir_python.py
tests/test_plot/test_plot_results.py

@muxator muxator closed this Sep 13, 2024
@muxator muxator deleted the remove-pytest-from-mypy-in-tox-ini branch September 13, 2024 09:18
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.

1 participant