tomshudson / swspy Goto Github PK

Package for conda distribution. Currently, packaging fails on tests for multiple OS.

I have just forked and cloned the repo to write an updated teleseismic splitting examples and I get the following error

AssertionError: Failed in nopython mode pipeline (step: convert to parfors)
Dimension mismatch for (Var($396return_value.4, split.py:424), Var($390build_tuple.3, split.py:424))

This is tracing back to line 6 in init.py

from .split import *

Which gets an error here :

~/Programs/swspy/swspy/splitting/split.py in <module>
    370 
    371 @jit((float64[:], float64[:], int64[:], int64[:], int64, int64, int64, float64, float64, float64[:,:,:], float64[:,:,:]), nopython=True, parallel=True)
--> 372 def _phi_dt_grid_search(data_arr_Q, data_arr_T, win_start_idxs, win_end_idxs, n_t_steps, n_angle_steps, n_win, fs, rotate_step_deg, 

which appears to be stemming from various compiler tests in numba.

This also appears to be the same as the build error currently showing (for me at least) on the repo homepage.

I am running python 3.10.9 and numba 0.56.4.

I've had a fiddle with the test harness (see #12). We now test on windows are part of the creation of pull requests. However, (some of) the notebooks fail on windows. Looks like a path error for reading the data. We should probably make sure we are using the new path objects so we don't get slashes the wrong way around (if that's what's going on).

Currently it looks like some of the places in the API relating to files or paths expect strings which we then manipulate / open using the low(ish) level os functions. It's probably more flexible to at least permit the new Path objects (from pathlib) and/or file like objects (so the caller can do the opening and, e.g. get data out of a compressed tar file or keep everything in memory).

Would need to be thought about (I've not looked at everything) and I think this is doable in a backwards compatible manner so shouldn't block any release.

The notebook testing pull request also enables tests on (new) pull requests. This should give a nice green tick that can be used to help to decide when to merge. It's probably worth enforcing this via the branch protection stuff on github (so changes to main only go via pull requests, and so all tests pass). Doesn't avoid the cheap hack of turning off a test we cannot fix, but at least we'll know we're doing it.

It's probably also good practice to have the test system enable linting (style checks). But before we do that we should fix any lint errors in the code first. And before that we should decide on the rules to enforce. Ultimately we'll need something like the following (which is a lax example) in the github workflow:

    - name: Lint with flake8
      run: |
        python3 -m pip install flake8
        # stop the build if there are Python syntax errors or undefined names
        flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics
        # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide
        flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics

unless we want to go hard-core with black?

Documenting how to contribute somewhere is probably a good thing too (do we need a contributors code of conduct?)

We should probably add some test coverage reporting. And more tests. What else?

There are a few bits and bobs we should look at to improve the testing:

• numba parallelisation of _phi_dt_grid_search() included. This should be tested (to the extent it can be) in the github actions
• Angular errors (e.g. for calculated fast directions) can appear large for small errors (e.g. 1 and 359 degrees). This could benefit from a helper function wrapping numpy's assert helpers. This could live in a new module (swspy.test, say).
• turn on coverage testing. Then use this to decide what else needs testing.

We have a task automation failure on github for 1.0.4 and nothing newer than 1.0.2 on pypi. Looks like a sed issue on macOS.

Current good practice (and the default on git and github) is to call the default branch 'main' not 'master' due to the connotations of 'master'. We should probably update our default branch name. See https://github.com/github/renaming for a how to.