Package / Actions Cleanup - Continuation of #435 by simmsa · Pull Request #436 · MHKiT-Software/MHKiT-Python

simmsa · 2026-02-25T00:22:20Z

This is a continuation and refinement of #435

Highlights:

From #435

Actions version updates:
- checkout - v4 to v6
- setup-python - v2 to v6
cdip fixes

Dependencies

Module Dependencies

Pin pandas below version 3.0
- There are some subtle bugs related to underlying pandas changes that we should address and possibly fix that are out of scope of this PR.
Refactor dependencies to move xarray, netcdf, and h5 dependencies into all
- This helps standardize the versions for all modules, and behind the scenes xarray typically requires both netcdf and h5.

Conda and Conda-Forge Dependency Definitions

Move conda forge environment file into environment-dev.yml and create new conda-forge-build job to test the conda-forge specific no deps build

Actions

Linting

Add filter to only run black on files that have changed
- Intent is to reduce noise in the tests. It is a non goal to have all files share the same formatting, we are just trying to enforce formatting on new or changed code
Add Development Installation section to readme detailing pip, conda, and conda forge installation pathways

Coveralls

Pin coveralls action at v2
Adds a switch at the top of main.yml to toggle reporting coveralls upload failures as test failures. Coveralls failures are of the 500 variant, Error: Bad response: 530 error code: 1016 per: https://status.coveralls.io/ and this implements the recommended fail-on-error: false fix

Update - Outage will be extended for at least several more hours. To avoid further disruption to your CI workflows, we recommend employing the fail-on-error: false input option available with all official coveralls integrations. See: https://docs.coveralls.io/integrations#official-integrations.
Feb 24, 2026 - 19:28 PST

We can incorporate this into #435 if that makes sense. My main goal here is to understand what changes are necessary in #435

There is a bug where something in requiring pyarrow that is likely related to pandas 3.0. Adding pyarrow as a dependency is a reasonable fix, but managing the pyarrow version should be handled by pandas and not mhkit. This pins pandas below 3.0 to see if pyarrow dependencies are caused by including >= 3.0 somewhere.

``` prepare-nonhindcast-cache The 'defaults' channel might have been added implicitly. If this is intentional, add 'defaults' to the 'channels' list. Otherwise, consider setting 'conda-remove-defaults' to 'true'. ```

simmsa · 2026-03-09T18:36:48Z

@jmcvey3

It's fine by me to keep this separate and close 435. We want to open another PR to specifically address pandas 3.0?

Yes, I think we should open another PR for pandas 3.0. I was a little surprised that we needed to explicitly include pyarrow as a dep for mhkit. Seems like that should be a part of pandas requirements and not something MHKiT should have to manage.

simmsa · 2026-03-09T18:40:23Z

Coveralls is operational now: https://status.coveralls.io/

I'm turning off the flag that ignores coveralls errors and this should pass and report to coveralls now...

akeeste · 2026-03-09T18:40:32Z

@simmsa is this ready for review? I was in the midst of reviewing but see more commits coming in. Please convert to a draft if not ready yet.

simmsa · 2026-03-09T18:44:05Z

@simmsa is this ready for review? I was in the midst of reviewing but see more commits coming in. Please convert to a draft if not ready yet.

On the whole this is ready.

As a sanity check, the last push just verifies that changes made here correctly report to coveralls when it IS operational.

simmsa · 2026-03-09T19:19:55Z

@akeeste this is good to go, apologies for the late push. Tests are passing with coveralls.

.github/workflows/main.yml

akeeste · 2026-03-12T19:54:03Z

.github/workflows/main.yml

-        uses: actions/checkout@v4
+        if: github.event_name == 'pull_request'
+        uses: actions/checkout@v6

      - name: Check for changes in wave/io/hindcast
        id: changes
+        if: github.event_name == 'pull_request'
        uses: dorny/paths-filter@v3
        with:
          filters: |
            wave_io_hindcast:
              - 'mhkit/wave/io/hindcast/**'
              - 'mhkit/tests/wave/io/hindcast/**'

-      - id: hindcast-logic
+      - name: Determine if hindcast tests should run
+        id: hindcast-logic
+        if: github.event_name == 'pull_request'
        run: |


Pushes also need to run the hindcast tests. See above comment. If we're getting false positives, then we should fix path-filter instead of limiting these tests to only PRs

The terminology is a bit ambiguous here from git so let me make sure I understand the intent.

There are 3 situations I see (I'm likely missing some):

A developer is working on module code not related to hindcast

A developer is working on infra/actions/package code

A developer is working on hindcast code

For 1 I don't think we should ever run the hindcast tests. They are too prone to failure, even for merges into develop. They add "noise" to the tests.

For 2, I think they should run ideally once, and maybe this is after a merge into develop, but this is the hardest case to figure out the right thing to do. This is likely when we would figure out the tests are failing because of some external change and need to intervene, but "when" is the question here.

For 3. The tests need to run every time a change is pushed, but don't necessarily need to run on merge, but maybe a merge run doesn't hurt.

@akeeste, what is your opinion here?

All three of those categories fall under PRs but runs on merges fall into a different category to me. Also I believe the hindcast tests were updated to only ever actually hit the API on main. We'd need to double check, but if I recall correctly, cached data was always used for PRs and maybe merges into main.

The hindcast tests should be very robust when we're not hitting the API, so perhaps we always run the hindcast tests, but only hit the API on merges into main or for PRs when the hindcast code is changed

@akeeste, sounds good. I reverted this to the original implementation because I don't think changes here added any value, and if anything changes made this more confusing.

.github/workflows/main.yml

environment-dev.yml

akeeste · 2026-03-12T20:21:47Z

README.md

+**Conda development** (minimal conda + pip resolves deps):
+
+```bash
+conda env create -f environment.yml
+conda activate mhkit-env
+pip install -e ".[all,dev]"
+```


Is this really a good use case to support and test? What's the value in testing a combined pip/conda install? I recall that the various hdf5/h5 packages are problematic, but if a pure pip install works and a pure conda install works, where will this method be used?

Regardless if we remove one of the .yml files all together, I think one of them should be bumped to the mhkit/tests directory so the user install method is clear

This is @jmcvey3 recommended conda install method, and seems like a middle ground to me between a pip install -e ".[all]" and the pinned conda-forge dependency install.

This supports a user or developer who wants to use conda to manage the environment and the h5 binaries but use pip to install everything else.

I'd think of this as a "power user" or "conda developer" install basically.

I will probably migrate to using this locally to develop as sometimes conda-forge deps with other libraries and the solver can be really slow.

That's fair that there is a use case here, but is it significant enough to warrant increasing test costs by 50%? Do the package versions installed by pure conda, pure pip and conda+pip differ enough to warrant the extra development cost?

@akeeste Fair question. If we offer a specific installation method it seems wise to test that installation method to verify it works. I don't think that this adds too much extra compute time, but happy to do whatever makes sense here.

To clarify, for the full pip install, are we using venv for environment control? I'm well versed in conda now, so I use it to handle environments and python versioning, even if I don't install conda-specific packages.

If the full pip install is functional, then a.) yes I agree this test case is redundant, and b.) we can remove the h5 binaries from the basic conda environment.yml file.

@jmcvey3, this seems reasonable to me. I will refactor accordingly.

.github/workflows/main.yml

jmcvey3 · 2026-03-13T18:03:24Z

.github/workflows/main.yml

+        shell: bash -l {0}
+        run: |
+          conda env create -f environment-dev.yml
+          conda activate mhkit-dev-env


So, this "env create" line (319) is actually redundant after the "Setup Miniconda" routine in line 304. You just need line 320 here is activate the environment.

@simmsa @akeeste So actually yeah, we're testing the full pip install using a conda environment every time. If we want a pure pip install, then we need to use venv as the package manager.

@jmcvey3 good call on venv over conda for pip install testing. This is definitely a gap in the documentation and tests.

I will work to refactor accordingly including updating the README documentation to recommend venv for pip installs and updating the tests.

So, this "env create" line (319) is actually redundant after the "Setup Miniconda" routine in line 304. You just need line 320 here is activate the environment.

Good find here, also. Will fix...

…ents

Previously we had `python>=3.10,<3.13` but this was likely legacy from the conda-forge version of the conda config. This removes the ambiguous python version, which relied on the solver, to a deterministic version with the intent of providing reproducible builds for conda users.

jmcvey3 · 2026-03-17T16:50:16Z

README.md

+**Pip development** (no conda):
+
+```bash
+pip install -e ".[all,dev]"


Just adding a note to create and activate a venv environment here

Good call, will update accordingly. LMK if other areas don't make sense here either, or could be clarified.

jmcvey3 and others added 30 commits February 19, 2026 09:01

Update year in license file

8648bff

fix upcrossing.py:upcrossing() for pandas 3.0

8862c89

lint and black formatting for the last commit

70f58ce

Clean up package installation, remove pandas 'd' time warning

b0d41d9

Update github actions versions, install dependencies for hindcast prep

d9f98b0

Square up caching code to make it easier to read. Room for improvement

a5149ed

Remove all the extra code in cdip.py

b3ae816

Remove requirements-dev file, add black to toml reqs

b8edd87

Merge branch 'develop' into package-cleanup

81c14b0

Fix refactoring of cdip file

9f7b92e

Create second environment file to test conda-forge package

d88b07f

Dev: Move conda-forge env file into the root dir

c8dd965

Actions: Refine conda build name for user facing installs

9ff07e8

Actions: Add conda-forge build/test steps

59f30e6

Actions: Remove unnecessary miniconda setup from single job

ddcd1e8

Actions: Remove no deps from user-conda install

3db1575

Actions: Update coveralls with new build step names

54ab157

Docs: Add development installation section

335e7de

Actions: Temp support actions on different branch

ab464e0

Actions: Only run black on changed files

b0f799d

Actions: Add comment above set-os code to clarify purpose

e557e71

Actions: Only run all OS on PR's into main

540d289

Actions: Use stricter logic to only run hindcast tests or pull requests.

88e2182

Dev: Remove adhoc branch for actions test

79f1b54

Dev: Revert unnecessary single to double quote changes

fc9f4a0

Actions: Add python 3.10 to test wheel build

05364cc

Actions: Fix "defaults" warning:

daf9c20

``` prepare-nonhindcast-cache The 'defaults' channel might have been added implicitly. If this is intentional, add 'defaults' to the 'channels' list. Otherwise, consider setting 'conda-remove-defaults' to 'true'. ```

Dev: Format with black

8215636

Actions: Fix, specify conda-forge as channel

ddf5b88

akeeste mentioned this pull request Mar 9, 2026

Package and testing updates and fixes #435

Closed

Actions: Fail actions if coveralls is down

92fdb34

akeeste requested changes Mar 12, 2026

View reviewed changes

akeeste mentioned this pull request Mar 12, 2026

D3D updates #428

Open

simmsa added 5 commits March 12, 2026 15:20

Actions: Run tests on all operating systems

4cae927

Dev: Change conda dev env name to mhkit-dev-env

0e806d7

Actions: Clarify hindcast should run logic

cfad06e

Actions: Remove pull request guard for should-run-hindcast

5945f23

Actions: Fix, use correct conda forge env name

e4b90e1

akeeste reviewed Mar 13, 2026

View reviewed changes

.github/workflows/main.yml Outdated Show resolved Hide resolved

simmsa added 4 commits March 13, 2026 10:06

Actions: Revert to use 'develop' jobs os matrix

9f12b4e

Actions: Revert hindcast run determination to original code

8f5a407

Actions: Ensure that apt packages are up to date

1718972

Actions: Add apt-get update for module tests

a6617da

jmcvey3 reviewed Mar 13, 2026

View reviewed changes

simmsa added 9 commits March 13, 2026 12:31

Actions: Remove conda-user-build job

3c2c6b1

Actions: Define single source variables for miniconda

a4efc63

Docs: Update marine energy terminology

0d2ea95

Docs: Add venv recommendation and clarify how to use virtual environm…

a736b27

…ents

Actions: Spec working directory for pip/venv install

abcfa7f

Actions: Simplify venv usage for pip install

9ca1182

Actions: Fix miniconda tar config syntax

f147e81

Env: Remove hdf5 binaries in favor of pip installing libs

1b26331

jmcvey3 reviewed Mar 17, 2026

View reviewed changes

Conversation

simmsa commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

From #435

Dependencies

Module Dependencies

Conda and Conda-Forge Dependency Definitions

Actions

Linting

Coveralls

Uh oh!

simmsa commented Mar 9, 2026

Uh oh!

simmsa commented Mar 9, 2026

Uh oh!

akeeste commented Mar 9, 2026

Uh oh!

simmsa commented Mar 9, 2026

Uh oh!

simmsa commented Mar 9, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simmsa Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akeeste Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

simmsa commented Feb 25, 2026 •

edited

Loading

simmsa Mar 12, 2026 •

edited

Loading

akeeste Mar 13, 2026 •

edited

Loading