Test Setup Review

[dostuffthatmatters]

This issue is related to the review process for JOSS: https://github.com/openjournals/joss-reviews/issues/6368

You already have a suite of PyTest test cases which is very good. All tests passed on my system on the first try.

• [x] Use a more established directory structure

I would recommend moving the file test_calculations.py and the directory test_files into a tests directory. Maybe also split up the test file into multiple test files - this is quite stylistic though. This repo follows a widely used project structure: https://github.com/navdeep-G/samplemod

README.md
...
tests/
    test_files/
    test_io.py
    test_conversions.py
    test_calculations.py

• [x] Add pytest to CI

You should run the tests using GitHub actions. If you choose to use PDM, you can simply use Pip to install the dependencies inside the CI and don't have to bother with setting up Conda/PDM/Pipenv/Poetry.

• [x] Run tests for different python versions (matrix)

The GitHub CI supports running tests for multiple Python versions in parallel

• [x] Run the end to end test on your test data in the CI

I.e. running qaqc_single_station.py using the default config. You would have to add an option to skip/do the corrections without a user input via keyboard. You can pipe a stdin to your script using echo 0 | python qaqc_single_station.py.

[cwdunkerly]

Finished all tasks, I split the end-to-end test and the pytest into separate workflows, testing the end-to-end across the full matrix of operating systems was consuming a lot of action time.

[dostuffthatmatters]

Finished all tasks, I split the end-to-end test and the pytest into separate workflows, testing the end-to-end across the full matrix of operating systems was consuming a lot of action time.

Great!

I think it is sufficient it you test the package on ubuntu-latest. Linux CI workers typically spin up way faster than MacOS or Windows workers. You can cache pip dependencies, and - if applicable - also use the pyproject.toml file as a dependency list for pip.

Below you can find a few changes I suggest for your .github/workflows/ci_pytest.yml. I included the test step for qaqc_single_station.py because then you can save some CI time in installation.

jobs:
  run-tests:
    strategy:
      fail-fast: false
      matrix:
-       os: [ubuntu-latest, macos-latest, windows-latest]
        python-version:
          - "3.9"
          - "3.10"
          - "3.11"
          - "3.12"

    name: run_tests
-   runs-on: ${{ matrix.os }}
+   runs-on: ubuntu-latest

    steps:
      - name: checkout_code
        uses: actions/checkout/@v4

      - name: setup_python
        uses: actions/setup-python@v5
        with:
          python-version: ${{ matrix.python_version }}
+         cache: 'pip'
+         cache-dependency-path: 'pyproject.toml'
      - name: install_dependencies
        run: |
          pip install -r pyproject.toml

      - name: run_pytest
        run: python -m pytest

+     - name: run_qaqc_single_station
+       run: echo 0 | python qaqc_single_station.py

[cwdunkerly]

@dostuffthatmatters That makes sense, thank you for the suggestion. I do have one question:

I understand the advantages of PDM/pyproject.toml, however when I used your setup as written I got the following errors:

Run pip install -r pyproject.toml

Defaulting to user installation because normal site-packages is not writeable

ERROR: Invalid requirement: '[project]' (from line 1 of pyproject.toml)

Error: Process completed with exit code 1.

I saw that other example .toml files had an initial '[build-system]' section, so I added that, but the error became:

Run pip install -r pyproject.toml

Defaulting to user installation because normal site-packages is not writeable

ERROR: Invalid requirement: '[build-system]' (from line 1 of pyproject.toml)

Error: Process completed with exit code 1.

Are you able to offer any advice? I was able to install the environment locally using PDM when I first created the .toml

EDIT:

I believe I need to use "uses: pdm-project/setup-pdm@v4", I apologize about the unnecessary ping. I'll get it converted over to that setup later this week.

[dostuffthatmatters]

Hi @cwdunkerly,

apologies, I gave you the wrong command. pip install . installs the dependencies from the pyproject.toml file. The command pip install -r filename only works for requirements.txt files. No need for pdm-project/setup-pdm@v4.

Also, in your current pyproject.toml, you should replace the line requires-python = ">=3.9.*" with requires-python = ">=3.9.0,<4.0". The former doesn't work because >=/<= need exact versions, not stars.