fix to pvlib/spa.py for issue #2077

[dgapitts]

• [X] PR to fix issues #2077
• [X] Tests added - details in this gist https://gist.github.com/dgapitts/a8749b5b9aa5413f350606314d48fb1b
• [X] I am familiar with the contributing guidelines

[kandersolar]

Thanks @dgapitts. Ideally the verification with expected values would be included as a new test in pvlib's test suite. Would you like to add that to this PR as well? I am happy to provide guidance if so.

[dgapitts]

Sure @kandersolar I can add the tests to pvlib/tests/test_spa.py I see there is already def test_julian_day_dt(self) but

• This only contains a single assert_almost_equal check - which was passing despite issue #2077
• Makes sense to me to add some or all of the following tests within def test_julian_day_dt(self)

tests = [
    ((2000, 1, 1, 12, 0, 0), 2451545.0),
    ((1999, 1, 1, 0, 0, 0), 2451179.5),
    ((1987, 1, 27, 0, 0, 0), 2446822.5),
    ((1987, 6, 19, 12, 0, 0), 2446966.0),
    ((1988, 1, 27, 0, 0, 0), 2447187.5),
    ((1988, 6, 19, 12, 0, 0), 2447332.0),
    ((1900, 1, 1, 0, 0, 0), 2415020.5),
    ((1600, 1, 1, 0, 0, 0), 2305447.5),
    ((1600, 12, 31, 0, 0, 0), 2305812.5),
    ((837, 4, 10, 7, 12, 0), 2026871.8),
    ((-123, 12, 31, 0, 0, 0), 1676496.5),
    ((-122, 1, 1, 0, 0, 0), 1676497.5),
    ((-1000, 7, 12, 12, 0, 0), 1356001.0),
    ((-1000, 2, 29, 0, 0, 0), 1355866.5),
    ((-1001, 8, 17, 21, 36, 0), 1355671.4),
    ((-4712, 1, 1, 12, 0, 0), 0.0),
]

[kandersolar]

I agree, adding all the cases makes sense. I suggest creating a new test function at the bottom of the test_spa.py file, rather than trying to integrate these new test cases with the existing code. The tests in test_spa.py use the old unittest style, but new tests should use pytest. Something like this: https://github.com/pvlib/pvlib-python/blob/main/pvlib/tests/test_shading.py#L229-L309

[dgapitts]

First, I wrote and tested the extra tests in a pytest separate file.

(.venv) ~/projects/pvlib-python bugfix/issue-2077 $ cat tests_issue_2077.py

import pvlib
import pytest

# Define the test cases as parameters
test_cases = [
    ((2000, 1, 1, 12, 0, 0), 2451545.0),
    ((1999, 1, 1, 0, 0, 0), 2451179.5),
    ((1987, 1, 27, 0, 0, 0), 2446822.5),
    ((1987, 6, 19, 12, 0, 0), 2446966.0),
    ((1988, 1, 27, 0, 0, 0), 2447187.5),
    ((1988, 6, 19, 12, 0, 0), 2447332.0),
    ((1900, 1, 1, 0, 0, 0), 2415020.5),
    ((1600, 1, 1, 0, 0, 0), 2305447.5),
    ((1600, 12, 31, 0, 0, 0), 2305812.5),
    ((837, 4, 10, 7, 12, 0), 2026871.8),
    ((-123, 12, 31, 0, 0, 0), 1676496.5),
    ((-122, 1, 1, 0, 0, 0), 1676497.5),
    ((-1000, 7, 12, 12, 0, 0), 1356001.0),
    ((-1000, 2, 29, 0, 0, 0), 1355866.5),
    ((-1001, 8, 17, 21, 36, 0), 1355671.4),
    ((-4712, 1, 1, 12, 0, 0), 0),
]

# Use pytest to parametrize the test
@pytest.mark.parametrize("inputs, expected", test_cases)
def test_julian_day(inputs, expected):
    result = pvlib.spa.julian_day_dt(*inputs, microsecond=0)
    assert result == expected, f"Failed for inputs {inputs}: expected {expected}, got {result}"


(.venv) ~/projects/pvlib-python bugfix/issue-2077 $ pytest tests_issue_2077.py
===================================================================================== test session starts ======================================================================================
platform darwin -- Python 3.12.4, pytest-8.3.3, pluggy-1.5.0
rootdir: /Users/davidpitts/projects/pvlib-python-orig
configfile: pyproject.toml
plugins: anyio-4.6.0
collected 16 items

tests_issue_2077.py ................                                                                                                                                                     [100%]

====================================================================================== 16 passed in 0.38s ======================================================================================

Next when I append this test to test_spa.py I got this NameError: name 'pytest' is not defined

(.venv) ~/projects/pvlib-python bugfix/issue-2077 $ pytest pvlib/tests/test_spa.py
===================================================================================== test session starts ======================================================================================
platform darwin -- Python 3.12.4, pytest-8.3.3, pluggy-1.5.0
rootdir: /Users/davidpitts/projects/pvlib-python-orig
configfile: pyproject.toml
plugins: anyio-4.6.0
collected 0 items / 1 error

============================================================================================ ERRORS ============================================================================================
___________________________________________________________________________ ERROR collecting pvlib/tests/test_spa.py ___________________________________________________________________________
pvlib/tests/test_spa.py:386: in <module>
    class NumbaSpaTest(unittest.TestCase, SpaBase):
pvlib/tests/test_spa.py:447: in NumbaSpaTest
    @pytest.mark.parametrize("inputs, expected", test_cases_issue_2207)
E   NameError: name 'pytest' is not defined
=================================================================================== short test summary info ====================================================================================
ERROR pvlib/tests/test_spa.py - NameError: name 'pytest' is not defined
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
======================================================================================= 1 error in 0.10s =======================================================================================

So to get around this, I add import pytest

(.venv) ~/projects/pvlib-python bugfix/issue-2077 $ pytest pvlib/tests/test_spa.py
===================================================================================== test session starts ======================================================================================
platform darwin -- Python 3.12.4, pytest-8.3.3, pluggy-1.5.0
rootdir: /Users/davidpitts/projects/pvlib-python-orig
configfile: pyproject.toml
plugins: anyio-4.6.0
collected 87 items

pvlib/tests/test_spa.py ..........................................sssssssssssssssssssssssssssssssssssssssssssss                                                                          [100%]

================================================================================ 42 passed, 45 skipped in 0.06s ================================================================================

[kandersolar]

Thanks @dgapitts, that's pretty much what I was imagining. A few tweaks are needed:

• de-indent the test so that it is stand-alone code at the end of the file, rather than included in the NumbaSpaTest class
• I think you will need to import pvlib at the top of the file
• the linter check will complain until all lines of code are 79 characters or shorter. Right now the line with the assert is too long. It's fine to just remove that long string; if the assertion ever fails, pytest will show basically the same information anyway.

[dgapitts]

I have pushed a commit (correct-test-indention-and-class) as requested @kandersolar.

Sorry for delay, I was having some issues with my local python venv.

Fortunately fixed/rebuilt now and tests working locally again:

(.venv) ~/projects/pvlib-python bugfix/issue-2077 $ pytest  pvlib/tests/test_spa.py
================================================================================================ test session starts =================================================================================================
platform darwin -- Python 3.12.4, pytest-8.3.3, pluggy-1.5.0
rootdir: /Users/davidpitts/projects/pvlib-python
configfile: pyproject.toml
plugins: anyio-4.6.2.post1
collected 102 items

pvlib/tests/test_spa.py ..........................................ssssssssssssssssssssssssssssssssssssssssssss................                                                                                 [100%]

=========================================================================================== 58 passed, 44 skipped in 0.06s ===========================================================================================

[cwhanse]

@dgapitts you can ignore the failure for that "Top Ranked Issues" action.

[dgapitts]

There is also a linter error:

Run git diff upstream/$GITHUB_BASE_REF HEAD -- "*.py" | flake8 --exclude pvlib/version.py --ignore E201,E241,E226,W503,W504 --max-line-length 79 --diff
Error: pvlib/tests/test_spa.py:430:1: E305 expected 2 blank lines after class or function definition, found 1
Error: pvlib/tests/test_spa.py:449:1: E302 expected 2 blank lines, found 1
Error: Process completed with exit code 1.

added commit fix-linting-errors-add-blank-lines to address this

[cwhanse]

2 blank lines before a function, one blank line at the end of a file.

[kurt-rhee]

thank you @dgapitts!