Skip to content

Fix failing CI workflows + guard delisted data integrity + bump actions to Node 24#154

Merged
JerBouma merged 1 commit into
JerBouma:mainfrom
dokson:fix/ci-workflows-and-delisted-hardening
Jun 4, 2026
Merged

Fix failing CI workflows + guard delisted data integrity + bump actions to Node 24#154
JerBouma merged 1 commit into
JerBouma:mainfrom
dokson:fix/ci-workflows-and-delisted-hardening

Conversation

@dokson

@dokson dokson commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

What

  • database_update.yml: restore the push branch filter to main. It was running on every branch, and the commit step failed on git checkout main (pathspec 'main' did not match) on non-main checkouts.
  • Node 24 bump (all 3 workflows): checkout@v6, setup-python@v6, setup-uv@v7, run-python-script-action@v1.8 — clears the Node 20 deprecation.
  • tests/test_invariants.py: invariant asserting equities delisted is strictly bool, so a NaN (which breaks ~equities["delisted"]) fails CI before merge.

Note

Per review feedback, the library filter is left untouched — bad delisted data should fail loudly. The invariant catches it at the data layer instead. Related: #153.

@dokson dokson force-pushed the fix/ci-workflows-and-delisted-hardening branch from 19ab3de to 1678103 Compare June 3, 2026 12:26
@JerBouma

JerBouma commented Jun 3, 2026

Copy link
Copy Markdown
Owner

The fact delisted contains a NaN is actually a database mistake and we should catch it before allowing it to be merged instead. I believe right now the database update GitHub Actions already prevents that.

I'd rather have the package fail when such a mistake is made than to allow the mistake through by changing the filtering option.

- database_update.yml: restrict the push trigger back to `main`. Without
  it the workflow ran on pushes to any branch and the commit step died on
  `git checkout main` ("pathspec 'main' did not match") since no local
  main exists on a non-main checkout.
- Bump all actions to Node 24 runtimes across the three workflows
  (checkout v6, setup-python v6, setup-uv v7, run-python-script-action
  v1.8), clearing the Node 20 deprecation warning.
- Add a cross-asset invariant asserting equities `delisted` is strictly
  bool, so a NaN (which breaks `~equities["delisted"]`) fails CI before
  merge instead of being silently filtered.
@dokson dokson force-pushed the fix/ci-workflows-and-delisted-hardening branch from 1678103 to 095fe92 Compare June 4, 2026 07:45
@dokson dokson changed the title Fix failing CI workflows + harden delisted filtering + bump actions to Node 24 Fix failing CI workflows + guard delisted data integrity + bump actions to Node 24 Jun 4, 2026
@dokson

dokson commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

Reverted the filter change. Heads up though: database_update.yml only sets delisted=False for new tickers, nothing keeps the column strictly boolean, which is how the NaN reached the bz2 and broke CI. So I added an invariant in tests/test_invariants.py asserting delisted is bool dtype — it fails on every PR before merge instead.

@JerBouma

JerBouma commented Jun 4, 2026

Copy link
Copy Markdown
Owner

That's perfect, if there is still a NaN in there I'll take it out manually.

@JerBouma JerBouma merged commit 6af7083 into JerBouma:main Jun 4, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants