MAINT: refine installation of core and test dependencies

[Sheila-nk]

Closes #76

[Sheila-nk]

After removing the installation of optional dependencies: scipy and matplotlib, self-tests are failing since we require both libraries to execute some of the doctests. They are technically optional, but we do need them for our testing purposes.

[Sheila-nk]

The goal of https://github.com/ev-br/scpdt/issues/76 is that self-tests pass in an environment without scipy or MPL. Tests which depend on either of them just need to be conditionally skipped, there is some usage of HAVE_SCIPY variable in test_testmod IIRC.

Oh! Okay, I was not sure how exactly to go about skipping them without using the --ignore cli option. Let me investigate why they're failing and try this approach instead.

[Sheila-nk]

@ev-br might need a bit of clarification The second flow is for self-tests only?

A minor inline question is optional, best acted upon in a separate PR if at all.

What do you mean by this?

[ev-br]

The second flow is for self-tests only?

Yup. Looking at https://github.com/ev-br/scpdt/blob/main/.github/workflows/pip.yml, two "Run testmod a scipy submodule" explicitly need scipy, the others do not?

The PosixPath vs Path change --- meant to mean this can be left out of this PR or added in, whichever is simpler.

[Sheila-nk]

Hey @ev-br, maybe you could review the new workflow before I make edits to the initial one? I've included the two tests from the initial workflow that do not require Scipy and MPL.

I also noticed that we're using older versions of the setup actions. If there's no specific reason for this, I can update them. Additionally, we could set up Dependabot to automatically create PRs when updates are available. Let me know what you think!

[ev-br]

TBH, I'm not sure what's going on with the GH workflows. On main, we have one which adds everything, including optionals. What we need is a second one which does not install MPL/SciPy and finishes with clear signal that some tests are skipped and the rest are OK. Why does the first one need tweaking and is the second one have optionals?

Side note: you might want to remove -s from pytest invocation in the new one so that it's less noisy.

Re dependabot: I'd rather not touch it with anything shorter than a bargepole.

[Sheila-nk]

On main, we have one which adds everything, including optionals. What we need is a second one which does not install MPL/SciPy and finishes with clear signal that some tests are skipped.

I honestly don't think we need a second workflow. In the initial workflow, pip install -e . here installs only the core dependencies i.e pytest and numpy. And the first test jobs are run without Scipy and MPL. The tests which require either of the optional dependencies are skipped. Then there is pip install -e '.[test]' here which installs scipy and matplotlib. And the rest of the tests carry on without skipping.

Why does the first one need tweaking and is the second one have optional?

No, the second one does not install optional dependencies. If we go ahead with the two-workflows approach, the two self-tests will need to be removed from the first workflow since they are being run in the same way as in the second workflow, except with a single pytest/python version

[Sheila-nk]

I have noticed that the 3rd test run is not running tests: Test testfile CLI after installation of the test dependencies. Will need to look into that.

[ev-br]

Ah, I did not realize how CI organized here! Please add some comments in the yml then. What we also want to make sure though, we want full self-tests pytest --pyargs scpdt run both with optionals and without them. Whether it's a single workflow or two, up to you.

[Sheila-nk]

There's this error locally and on GH: https://github.com/ev-br/scpdt/actions/runs/8186212193/job/22384153598?pr=131#step:8:149 I'm unfamiliar with the matplotlib library, so maybe you can help? @ev-br The internet isn't much help and debugging hasn't provided any enlightenment.

[ev-br]

Hm. I do see something similar locally. However I don't think it's related to matplotlib:

$ pytest --pyargs scpdt

reports

FAILED scpdt/tests/test_skipmarkers.py::TestSyntaxErrors::test_invalid_python - AssertionError: assert <class 'AttributeError'> is TypeError
FAILED scpdt/tests/test_skipmarkers.py::TestStopwords::test_bogus_output - doctest.UnexpectedException: <DocTest stopwords_bogus_output from none:0 (1 example)>
FAILED scpdt/tests/test_testmod.py::test_stopwords - assert 1 == 0

But rerunning individual files does not:

$ pytest scpdt/tests/test_skipmarkers.py::TestStopwords::test_bogus_output
===================================================== test session starts =====================================================
platform linux -- Python 3.10.0, pytest-8.0.2, pluggy-1.4.0 -- /home/br/mambaforge/envs/scpdt2/bin/python3.10
cachedir: .pytest_cache
rootdir: /home/br/sweethome/proj/scipy/refguide_check_2/scpdt
configfile: pyproject.toml
plugins: scpdt-0.1
collected 1 item                                                                                                              

scpdt/tests/test_skipmarkers.py::TestStopwords::test_bogus_output PASSED                                                [100%]

====================================================== 1 passed in 0.44s ======================================================

So it's not matplotlib, it's us unfortunately. Let me try bisecting it.

[ev-br]

$ git bisect bad
aa5e4878cf099d436c1b5895f79da0d66eb17f30 is the first bad commit
commit aa5e4878cf099d436c1b5895f79da0d66eb17f30
Author: Sheila-nk <sheilankw@gmail.com>
Date:   Tue Mar 5 10:43:21 2024 +0300

    skip self tests that require scipy or matplotlib

 .github/workflows/pip.yml                |  5 ++++-
 scpdt/tests/test_pytest_configuration.py | 20 ++++++++++++++++++--
 scpdt/tests/test_skipmarkers.py          |  8 ++++++++
 scpdt/tests/test_testmod.py              | 12 +++++++++++-
 4 files changed, 41 insertions(+), 4 deletions(-)

The good/bad criterion is $ pytest --pyargs scpdt -x fails or passes.

[Sheila-nk]

Decided to start with a new branch from main and gradually add changes from this branch. Everything works fine until I try to import matplotlib:

try:
    import matplotlib    # noqa
    HAVE_MATPLOTLIB = True
except Exception:
    HAVE_MATPLOTLIB = False

I can skip tests when matplotlib is not installed. But once I install it, the tests fail. And when I remove the import, they pass. 😆 Trying to see what that is about. Any ideas?

[Sheila-nk]

scpdt/tests/test_pytest_configuration.py::test_stopword_cases SKIPPED (need matplotlib)       [ 40%]
scpdt/tests/test_pytest_configuration.py::test_local_file_cases PASSED                        [ 42%]
scpdt/tests/test_runner.py::test_single_failure PASSED                                        [ 44%]
scpdt/tests/test_runner.py::test_exception PASSED                                             [ 46%]
scpdt/tests/test_runner.py::test_get_history PASSED                                           [ 48%]
scpdt/tests/test_runner.py::TestDebugDTRunner::test_debug_runner_failure PASSED               [ 51%]
scpdt/tests/test_runner.py::TestDebugDTRunner::test_debug_runner_exception PASSED             [ 53%]
scpdt/tests/test_skipmarkers.py::TestSyntaxErrors::test_invalid_python FAILED                 [ 55%]
scpdt/tests/test_skipmarkers.py::TestSyntaxErrors::test_invalid_python_plus_skip PASSED       [ 57%]
scpdt/tests/test_skipmarkers.py::TestSyntaxErrors::test_invalid_python_pseudocode PASSED      [ 59%]
scpdt/tests/test_skipmarkers.py::TestPseudocodeMarkers::test_pseudocode_markers PASSED        [ 61%]
scpdt/tests/test_skipmarkers.py::TestStopwords::test_bogus_output FAILED                      [ 63%]
scpdt/tests/test_skipmarkers.py::TestMayVary::test_may_vary PASSED                            [ 65%]
scpdt/tests/test_skipmarkers.py::TestMayVary::test_may_vary_source PASSED                     [ 67%]
scpdt/tests/test_skipmarkers.py::test_SKIPBLOCK PASSED                                        [ 69%]
scpdt/tests/test_testfile.py::test_one_scipy_tutorial XFAIL (needs the scipy repo at a fi...) [ 71%]
scpdt/tests/test_testfile.py::test_linalg_clone PASSED                                        [ 73%]
scpdt/tests/test_testmod.py::test_module PASSED                                               [ 75%]
scpdt/tests/test_testmod.py::test_module_vanilla_dtfinder PASSED                              [ 77%]
scpdt/tests/test_testmod.py::test_stopwords FAILED  

In the above run, I skipped test_stopword_cases using the HAVE_MATPLOTLIB variable. It was skipped and the rest of the tests that require matplotlib and have no skip marker failed as expected.

@pytest.mark.skipif(not HAVE_MATPLOTLIB, reason='need matplotlib')
    def test_stopword_cases(pytester):
        """Test that pytest uses the DTParser for doctests."""
        path_str = stopwords_cases.__file__
        python_file = PosixPath(path_str)
        result = pytester.inline_run(python_file, "--doctest-modules")
>       assert result.ret == pytest.ExitCode.OK

But when I installed matplotlib, all tests that require matplotlib failed. And this is the error:

Failed example:
    import matplotlib.pyplot as plt; plt.xlim([1, 2])
Exception raised:
    Traceback (most recent call last):
      File "/Users/sheilakahwai/mambaforge/lib/python3.10/doctest.py", line 1350, in __run
        exec(compile(example.source, filename, "single",
      File "<doctest stopwords_bogus_output[0]>", line 1, in <module>
        import matplotlib.pyplot as plt; plt.xlim([1, 2])
      File "/Users/sheilakahwai/Documents/Work/scpdt/venv/lib/python3.10/site-packages/matplotlib/pyplot.py", line 1958, in xlim
        ax = gca()
      File "/Users/sheilakahwai/Documents/Work/scpdt/venv/lib/python3.10/site-packages/matplotlib/pyplot.py", line 2540, in gca
        return gcf().gca()
      File "/Users/sheilakahwai/Documents/Work/scpdt/venv/lib/python3.10/site-packages/matplotlib/figure.py", line 1658, in gca
        return ax if ax is not None else self.add_subplot()
      File "/Users/sheilakahwai/Documents/Work/scpdt/venv/lib/python3.10/site-packages/matplotlib/figure.py", line 782, in add_subplot
        ax = projection_class(self, *args, **pkw)
      File "/Users/sheilakahwai/Documents/Work/scpdt/venv/lib/python3.10/site-packages/matplotlib/axes/_base.py", line 670, in __init__
        self._init_axis()
      File "/Users/sheilakahwai/Documents/Work/scpdt/venv/lib/python3.10/site-packages/matplotlib/axes/_base.py", line 812, in _init_axis
        self.xaxis = maxis.XAxis(self, clear=False)
      File "/Users/sheilakahwai/Documents/Work/scpdt/venv/lib/python3.10/site-packages/matplotlib/axis.py", line 2295, in __init__
        super().__init__(*args, **kwargs)
      File "/Users/sheilakahwai/Documents/Work/scpdt/venv/lib/python3.10/site-packages/matplotlib/axis.py", line 676, in __init__
        self.label = mtext.Text(
      File "/Users/sheilakahwai/Documents/Work/scpdt/venv/lib/python3.10/site-packages/matplotlib/text.py", line 155, in __init__
        self.update(kwargs)
      File "/Users/sheilakahwai/Documents/Work/scpdt/venv/lib/python3.10/site-packages/matplotlib/text.py", line 197, in update
        kwargs = cbook.normalize_kwargs(kwargs, Text)
      File "/Users/sheilakahwai/Documents/Work/scpdt/venv/lib/python3.10/site-packages/matplotlib/cbook.py", line 1783, in normalize_kwargs
        for canonical, alias_list in alias_mapping.items()
    AttributeError: type object 'Text' has no attribute 'items'

When I remove the matplotlib import, all the tests pass.

[ev-br]

1) on main with MPL, do tests pass? 2) on main + a single conditional skip, do tests pass?

[Sheila-nk]

on main with MPL, do tests pass?

Yes, all tests pass.

on main + a single conditional skip, do tests pass?

No. Interestingly, attempting to import MPL causes ALL tests requiring MPL to fail, even before I apply skip markers to any of the tests. It's quite strange.

The current workaround I have found is conditionally importing MPL at the test level.

[ev-br]

What is the MPL version and what happens if you downgrade it?

[ev-br]

OK, it does not look dependent on the MPL version in the end. In two envs with matplotlib 3.7.1 and 3.8.1, using the following patch seems to work:

$ git diff
diff --git a/scpdt/tests/test_skipmarkers.py b/scpdt/tests/test_skipmarkers.py
index d68e2b8..64c82f0 100644
--- a/scpdt/tests/test_skipmarkers.py
+++ b/scpdt/tests/test_skipmarkers.py
@@ -4,11 +4,20 @@ import pytest
 from ..impl import DTConfig, DTParser, DebugDTRunner

+try:
+    import matplotlib.pyplot as plt    # noqa
+    HAVE_MATPLOTLIB = True
+except Exception:
+    HAVE_MATPLOTLIB  = False
+
+HAVE_MATPLOTLIB = False
+
 class TestSyntaxErrors:
     """Syntax errors trigger a doctest failure *unless* marked.

     Either mark it with +SKIP or as pseudocode.
     """
+    @pytest.mark.skipif(not HAVE_MATPLOTLIB, reason="MPL optional")
     def test_invalid_python(self):
         # This string raises
         # TypeError("unsupported operand type(s) for +: 'int' and 'tuple'")

Note that here I'm importing matplotlib.pyplot instead of matplotlib itself.

Why?

https://github.com/matplotlib/matplotlib/issues/26827 looks related. I have no clue of what's going on there, apart from "something is weird in the way the top-level matplotlib package is organized". Furthermore, note that if we import matplotlib itself,

$ pytest --pyargs scpdt   # breaks
$ pytest scpdt/tests         # breaks
$ pytest .                            # breaks
$ pytest scpdt/tests/test_skipmarkers.py     # does not break!

So something is weird in how the matplotlib namespace imports but it's usually OK. Something is weird in the way pytest imports things but it's usually OK. When these two weirdnesses collide, they conspire.

Can you check if this works on your system Sheila?

[Sheila-nk]

Can you check if this works on your system Sheila?

Yes, works both locally and on GH! 👍

[ev-br]

Great! So can finish up this PR then?

[Sheila-nk]

Yes, I believe it's done. Maybe you can review it before I proceed to merge?

[ev-br]

LGTM now, thank you Sheila!