Test interp nominal lon

[JoranAngevaare]

Fix test for _interp_nominal_lon

This PR is some leftover work from #296 (and #300)

• I've added a dummy test - which is a slimmed down version of the example issue in replace_x_y_nominal_lat_lon does not work for > 360 lon coordinates #295.
• Add type-hints to Assume longitude is cyclic in _interp_nominal_lon #296
• Fix Adding new contributors to CITATION.cff #358

PR checks

• Closes replace_x_y_nominal_lat_lon does not work for > 360 lon coordinates #295, Adding new contributors to CITATION.cff #358
• Tests added
• Passes pre-commit run --all-files
• User visible changes (including notable bug fixes) are documented in whats-new.rst
• New functions/methods are listed in api.rst

[jbusecke]

Thanks for putting in more work into polishing this up @JoranAngevaare! I would much prefer to keep this in pure pytest, but I might also be missing something here.

[jbusecke]

I am not quite sure this will actually test correctly with pytest. I have only ever written funtions with assert statements. See https://docs.pytest.org/en/stable/getting-started.html

If you want to ensure that a certain Exceptio is raised you can wrap the statement with

with pytest.raises(...)
    ...

[jbusecke]

See my other comment. That will hopefully lead this to actually fail? Once that is the case I wouöd suggest to remove the "old way" logic to keep this test more compact

[JoranAngevaare]

@jbusecke of course value-errors result in failed tests and this syntax does trigger the tests:

(py39) aangevaare@surfclimahuge:~/software/xMIP$ pytest tests/test_preprocessing.py -k TestReplaceXYNominalLatLon
================================================= test session starts ==================================================
platform linux -- Python 3.9.0, pytest-8.2.2, pluggy-1.5.0 -- /home/aangevaare/miniconda3/envs/py39/bin/python
cachedir: .pytest_cache
rootdir: /home/aangevaare/software/xMIP
configfile: pyproject.toml
collected 140 items / 138 deselected / 2 selected

tests/test_preprocessing.py::TestReplaceXYNominalLatLon::test_old_fails PASSED                                   [ 50%]
tests/test_preprocessing.py::TestReplaceXYNominalLatLon::test_new_works PASSED                                   [100%]

=================================================== warnings summary ===================================================
xmip/preprocessing.py:4
  /home/aangevaare/software/xMIP/xmip/preprocessing.py:4: UserWarning: Import(s) unavailable to set up matplotlib support...skipping this portion of the setup.
    import cf_xarray.units  # noqa: F401

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
===================================== 2 passed, 138 deselected, 1 warning in 8.37s =====================================

But sure, let me write assert statements if that's more conform the way things are done for xmip

[JoranAngevaare]

@jbusecke as requested:

• I replaced the raise ValueError with assert 9faf75f.
• To show that it works explicitly, I pushed 0270f4b which reverts the logic, and shows that the old code would have raised an error.
• I think you asked me to remove the showcase of the old test. In that case, using a class is of course completely redundant so I wrote it as a single function in fda6fbc.

[jbusecke]

Thank you so much for iterating on this @JoranAngevaare . Really appreciate the ongoing work. I had a minor ? about s comment, but otherwise I think this is ready!

[jbusecke]

The tests are now passing, so this comment confuses me slightly. Is it a leftover?

[JoranAngevaare]

That's the message that the AssertionError will produce once failed, see for example the message in https://github.com/jbusecke/xMIP/actions/runs/9857864431/job/27217936984 (pytest also prints the lons, and lons_parsed so it looks a bit messy).

Of course, it shouldn't (and doesn't) fail so I could as well remove it

[jbusecke]

That would be awesome! Ill merge it right away.

[JoranAngevaare]

Great! I already pushed the commit

[jbusecke]

of course value-errors result in failed tests and this syntax does trigger the tests

Thanks for verifying this. And it obviously makes sense (Ill keep that in mind for future test designs, and thanks for teachibg me this!). I should have tested this myself (but am using gh on mobile so am a bit restricted here).