pyspectral icon indicating copy to clipboard operation
pyspectral copied to clipboard

Add a testing module to facilate testing in external modules

Open mraspaud opened this issue 3 months ago • 7 comments

In the making of https://github.com/pytroll/satpy/pull/3260, it has appeared some of the functionality of pyspectral needed mocking to avoid eg downloading srfs or luts from the internet during unit testing.

This mocking, or preferably patching with some realistic behaviour (eg synthetic luts), would be best placed in a "testing" module in pyspectral, with for example context managers like with fake_luts or with mocked_srfs.

mraspaud avatar Oct 13 '25 16:10 mraspaud

Would it be useful for the classes that use the LUTs to have something similar to a "dry_run" option? I guess that would have to allow a more global configuration to be useful for Satpy as the classes are used internal to the Satpy components being tested. Or is this such a specific use case (testing with pyspectral components and no downloading) that it should be testing specific. I guess there isn't much use of the features if they aren't real LUTs.

djhoese avatar Oct 13 '25 16:10 djhoese

I’m thinking without luts, rayleigh correction is not doable for now. When it comes to the SRFs, there is already the possibility to run using only the central wavelength of the band at hand, so for that case, an explicit switch could work. But still, a context manager to disable srf downloading temporarily would be useful in that case too I think.

mraspaud avatar Oct 14 '25 08:10 mraspaud

So I'm kind of wondering if both pieces of my Satpy PR should make their way to pyspectral. Pyspectral already has a way to disable reaching out to the internet so a context manager to disable it (maybe in pyspectral.testing) would make sense. This way if pyspectral every changes how it downloads things then it "just works" in Satpy.

The mocked classes in a context manager may not be more than an autospec that yields the mock object. It would be up to the user to take the mock object and add the functionality they want. Is that still that useful?

djhoese avatar Oct 14 '25 11:10 djhoese

I'm not sure the user would need to manipulate the mock object, unless we need to test with specific luts?

mraspaud avatar Oct 14 '25 13:10 mraspaud

I don't mock all functionality currently and a user may want specific handling or mimicking invalid values or something. If I don't return/yield the mock object then there isn't anything to return so it wouldn't hurt in my opinion to return it.

djhoese avatar Oct 14 '25 16:10 djhoese

Ok

mraspaud avatar Oct 15 '25 04:10 mraspaud

@mraspaud I started working on this and I'm not sure how useful this is to anyone else. My Calculator mock in my Satpy PR assumes xarray DataArrays with dask arrays underneath. It also mocks the import of the Calculator that Satpy uses in its own modules. Mocking the one inside pyspectral requires (I think) that my mock happen before anything else in the user's (Satpy's) tests otherwise it isn't mocking the already imported object of the user's modules.

I also wanted to add the autouse fixture for disabling downloads but that too can't be copied 1:1. You'd have to import a context manager from Pyspectral and then use that in your own conftest.py...or maybe importing a fixture would work, but it is a little awkward.

djhoese avatar Oct 29 '25 20:10 djhoese

I was thinking of decorator eg with mock_pyspectral_downloads(tle_to_return) that could monkey-patch the downloading process, are you saying that can't be done?

mraspaud avatar Nov 07 '25 12:11 mraspaud

What is tle_to_return? Pyspectral isn't downloading TLEs so what would that context manager be doing?

djhoese avatar Nov 10 '25 02:11 djhoese

tle_to_return would be the tle the mock download returns, so that to the test it would look like the downloaded TLE is the content of tle_to_return. Ie:

with mock_pyspectral_downoads(tle_to_return):
    downloaded_tle = pyspectral.fetch_tle("NPP")
    assert downloaded_tle == tle_to_return  # true

mraspaud avatar Nov 10 '25 10:11 mraspaud

Sure maybe that specific interface is slightly simplified by that calling structure, but the ones used in the Satpy tests that started this issue are classes with multiple methods which may have complex return values and operations. But what I said earlier still applies: a mock in a test is supposed to mock the imported instance of the object, not the original in-pyspectral import path. Otherwise, if the user's code has already imported pyspectral's object the mocking done by this helper context manager won't be used.

# user module - satpy.use_rayleigh
from pyspectral.rayleigh import Rayleigh


# test module

from satpy.use_rayleigh import RayleighUseCase

def test_rayleigh():
    with mock.patch("pyspectral.rayleigh.Rayleigh") as mock_ray:
        # this doesn't do anything, Rayleigh was already imported and "lives" in satpy.use_rayleigh"

    with mock.patch("satpy.use_rayleigh.Rayleigh") as mock_ray:
        # I think this works

In the above, we'd have to know the users import path where the pyspectral object is used. Otherwise things are dependent on import order.

djhoese avatar Nov 10 '25 15:11 djhoese

Ok, sorry about the naming (tle_to_return) I got confused and thought for a while we wanted to mock TLE downloads in pyorbital.

You make a perfectly valid point about import order, but I would argue that this shouldn't matter if we are mocking the internals of pyspectral that will not be imported by other libraries.

As an example, here is such a testing utility I created this morning for pyorbital (I was still confused, sorry!):

testing.py

from contextlib import contextmanager
from io import StringIO
from unittest.mock import patch


@contextmanager
def mock_tle_dl(new_tle):
    """Mock the tle downloading and replace the "fetched" tle with `new_tle`."""
    def string_open(*args, **kwargs):
        del args, kwargs
        return StringIO(new_tle)

    with patch("pyorbital.tlefile._get_internet_uris_and_open_method") as new_uri_and_open:
        new_uri_and_open.return_value = [None], string_open
        yield

test_testing.py

TLE_HDR = "NOAA 20 (JPSS-1)"
TLE_LINE1 = "1 43013U 17073A   25314.91926032  .00000182  00000+0  10707-3 0  9994"
TLE_LINE2 = "2 43013  98.7548 251.5721 0001435 133.2261 226.9035 14.19512458413508"
TLE_EXAMPLE = "\n".join((TLE_HDR, TLE_LINE1, TLE_LINE2))


def test_mock_tle_dl():
    """Test the tle download mocking context."""
    from datetime import datetime

    from pyorbital.orbital import Orbital
    from pyorbital.testing import mock_tle_dl
    with mock_tle_dl(TLE_EXAMPLE):
        orb = Orbital("NOAA-20")
        orb.get_position(datetime(2020, 2, 2, 2, 2))
        assert orb.tle._line1 == TLE_LINE1
        assert orb.tle._line2 == TLE_LINE2

As you can see, we inject the mocked behaviour in a private function, so that there is no chance import order/context matters.

So in the same fashion we can have a testing utility that mocks the luts download, possibly replacing it with synthetic/dummy lut data for the purpose of testing.

mraspaud avatar Nov 11 '25 10:11 mraspaud