Improve auto northing

[aclerc]

I noticed that the auto northing process can be a bit funny when the north correction is near 180deg. I realized the root cause might be that ruptures is not using circular math. In this PR I added a custom cost function for ruptures. Unfortunately I believe this slows down autonorthing a bit, but since overall it's gone from extremely slow to still extremely slow I think that's acceptable for now.

While I was in this code I took the opportunity to improve a few more things:

• refined the auto northing scoring a bit
• fixed up missing circular math in a few places, including the northing code itself, which means some test case answers change

I have re-run the smarteole example notebook, adding a print of the wind-up version to the top so it's easy to see if it needs a refresh

[samuelwnaylor]

Nice one thinking of the performance of the functions 👍
Thanks for adding useful comments 🙂

I've left some optional comments.

[samuelwnaylor]

Very much optional comment, but a naming suggestion to perhaps use unsigned vs signed,

i.e.

the existing...

def circ_median(angles: npt.NDArray, axis: int | None = None, *, range_360: bool = True) -> float | npt.NDArray:
    ...

could become...

def circ_median(angles: npt.NDArray, axis: int | None = None, *, return_range_format: Literal["unsigned", "signed"] = "unsigned") -> float | npt.NDArray:
    ...

[aclerc]

to me it's much less obvious what "signed" and "unsigned" would mean compared to "range_360". I think "range_360" isn't great though so open to changing it to something better

[samuelwnaylor]

Yeah makes sense, it's a tricky one to name well 😆

[samuelwnaylor]

Optional comment...

These tests relating to circ_median could be grouped into a class for readability. I could also be tempted to separate out the tests for those that have range_360=False into it's own method for readability too.

e.g.

class TestCircMedian:
    
    @staticmethod
    @pytest.mark.parametrize(
        ("angles", "expected"), 
        [
            ([0], 0),
            ([0, 20], 10),
            ...
        ]
    )
    def test_range360(angles: list, *, expected: float | None) -> None:
        ...


    @staticmethod
    @pytest.mark.parametrize(
        ("angles", "expected"), 
        [
            ([170, 180, 190], 180),
            ([350, 0, 10], 0),
            ([-10, 0, 10], 0),
        ]
    )
    def test_not_range360(angles: list, *, expected: float | None) -> None:
        ...

    @staticmethod
    def test_with_series() -> None:
        """Test that pandas Series work correctly."""
        angles = pd.Series([350, 0, 10, 5])
        result = circ_median(angles)
        assert isinstance(result, (float, np.floating))
        assert result == pytest.approx(2.5)

    @staticmethod
    def test_with_some_nan() -> None:
        ...

    @staticmethod
    def test_with_all_nan() -> None:
        ...

    @staticmethod
    def test_groupby() -> None:
        ...

    ...

    ...

[aclerc]

If the only advantage is readability then I'd leave them as they are, putting them in a class doesn't seem more readable to me. Granted as static methods you don't need to check for class context (I guess) but you do get extra indent by doing this, minor but it does matter for readability

[samuelwnaylor]

The main advantages in this scenario are...

• organization and grouping of tests logically, making the test suite easier to navigate and understand (here different aspects of the same feature are being tested, so grouping is convenient to a reader)
• simple to run all tests in the class, pytest -k TestCircMedian (or in your IDE)

[samuelwnaylor]

Could inform the user to call .fit() method?