Add Continuous Integration

[jatkinson1000]

Add continuous integration:

• black
• pydocstyle (numpy)
• mypy
• pylint
• pytest
• Run on various OS/versions

Separate yaml files for linting and testing.

Will close #20

[jatkinson1000]

@LiamPattinson I've spent a while getting this to conform to mypy, but this is the first time I've really used it this extensively in a project or with numpy, and there were a few difficult bits. Would you be willing to do a code review on this PR at all once I feel it's ready to go?

[LiamPattinson]

Can do, but I'm super-busy with work at the minute, so might not be able to look at it in detail for a little while. Feel free to ping me for any specific issues though. One thing I noticed at a glance is that pylint seems to be flagging up a lot of errors that are actually pretty reasonable stylistic choices. It might be worth disabling a bunch of the linting rules project-wide, or maybe switching out for something a bit more permissive like flake8.

[jatkinson1000]

Thanks @LiamPattinson and completely understandable.

Yeah - I've disabled the pylint checks for the time being whilst I develop but will add exceptions to specific lines where I feel I am correct.

The bit I most want reviewed is the mypy stuff.
e.g.

return val - scr

had to become

        # Ensure we return float, not np.ndarray
        # These 9 lines replace `return val-scr` so as to satisfy mypy --strict.
        # Should never be triggered in reality as h is type float.
        if type(val) is np.float_:
            val = val.item()
        if type(val) is float:
            return val - scr
        else:
            raise TypeError(
                f"f_root is attempting to return a {type(val)} type. "
                f"Was it passed an array of handicaps?"
            )

to satisfy mypy that it would never return an array. However, this feels very verbose.

Also I spent ages trying to get numpy to play nice. Eventually settled on using Union[float, np.float_, npt.NDArray[np.float_]] in a lot of places, but I am not sure when the best time to convert an np.float_ to a native python float is - at return, never, every function, only functions exposed to user?

[LiamPattinson]

Would return float(val) - scr be sufficient in this case? It's usually easier to just coerce types into whatever you need them to be than to handle every possibility individually. It'll raise a TypeError anyway if the user tries to give it a list/array/etc, though it'll take a bit of try...except and re-raising if you want it to have nicer error messages.

If its code which should work either on numpy arrays or built-in floats, I find the best solution is to convert to 0D arrays as standard:

# ArrayLike will catch scalars, np.ndarray, lists, tuples, etc
def myfunc(scalar_or_array: np.typing.ArrayLike) -> np.ndarray:
    # Ensure the input is converted to nd.array
    # If passed a scalar, it becomes a weird 0D array, but due to duck typing this
    # rarely matters in practice and it behaves just like a built-in scalar type.
    # Other options:
    # - np.asanyarray: converts to ndarray, but won't copy data if given an array view or something
    # - np.asfarray: shorthand for np.asarray(myarray, dtype=float)
    scalar_or_array = np.asarray(scalar_or_array)
    ... do the rest of the function ...

I don't actually use mypy or any other static type checker in my own work, and use type hints more as in-code documentation, so I can't say whether that'd be enough to keep mypy happy.

[LiamPattinson]

Sorry if this is stuff you already know, but the numpy typing docs have some stuff on mypy and some general advice for keeping type checkers happy. It says there that any functions that can return either scalars or arrays are currently typed as simply returning np.ndarray, even though numpy will always try to convert a 0D array to the underlying built-in type, so unfortunately you might need to follow a similar rule to keep mypy from complaining.

[jatkinson1000]

Yes, though to be honest I found the numpy typing docs fairly opaque... 😅
And the rule aboud 0D is interesting, because I'm finding 0D arrays are being returned as np.float64 rather than float - similar to issues I had in some of my work at BAS.
Using the plugin though.

On the topic of forcing float with float(val), sadly this doesn't appeas mypy as it then complains because val could theoretically be an array (even though we know it isn't) and float() is an invalid operation for an array -_-
At least this has made me hyper aware of how 'broken' python is 😅

Seem to be making progress though...
Although adding this may limit the versions of python I can run on it seems.

[jatkinson1000]

Thanks to comments from @LiamPattinson in #26 I have made some changes to remove mypy from the testing process to run only on linting.
Also separated out project dependencies to core and optional.
Testing on python 3.7-3.11, all of which pass.
As a result this PR will now close #26

Commits are a bit small and messy but... eehhhh

All 'legitimate' linting errors have been addressed.
Now need to address any linting errors that I feel are OK.

[jatkinson1000]

I'm going to merge this now as the base aim of adding CI has been met.

The fact that pylint is not passing hints at some of the code smelling a bit dodgy and requiring a refactor. This should be fixed when addressing some other issues (e.g. #24 ).

To encapsulate all of this I'll open a new issue for getting CI green.

[LiamPattinson]

Yeah, I think that's a sensible way of going about it. Better to clean up the code piece-by-piece as you're doing general refactoring than to spend hours polishing something you're just going to rewrite in a few weeks anyway.