Skip to content

CI: explicitly include defaults channel #1129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Feb 17, 2025
Merged

CI: explicitly include defaults channel #1129

merged 3 commits into from
Feb 17, 2025

Conversation

slivingston
Copy link
Member

This change is motivated by that of commit 814f414 (from PR #1128)

slivingston added a commit to slivingston/Slycot that referenced this pull request Feb 17, 2025
@slivingston
CI: explicitly include defaults channel
fd6be99 
Similar to changes from
python-control/python-control#1129
python-control/python-control#1128
@coveralls
Copy link

coveralls commented Feb 17, 2025 •
edited
Loading

Coverage Status

coverage: 94.744%. remained the same
when pulling 6c628f0 on slivingston:ci-add-defaults-miniforge
into 061749d on python-control:main.

@slivingston
CI: explicitly include defaults channel
3079721 
This change is motivated by that of commit 814f414
@slivingston slivingston force-pushed the ci-add-defaults-miniforge branch from 7499bd5 to 3079721 Compare February 17, 2025 00:38
slivingston added a commit to slivingston/Slycot that referenced this pull request Feb 17, 2025
@slivingston
CI: explicitly include defaults channel
03bff79 
Similar to changes from
python-control/python-control#1129
python-control/python-control#1128
@murrayrm
Copy link
Member

murrayrm commented Feb 17, 2025 •
edited
Loading

@slivingston Despite the claim that all checks completed, in fact the conda-based tests did not complete. See OS/BLAS test matrix run. It looks like the conda test matrix gets created but does not run.

The error is

Error when evaluating 'strategy' for job 'test-conda'. 
.github/workflows/os-blas-test-matrix.yml (Line: 277, Col: 15): 
Error parsing fromJson,.github/workflows/os-blas-test-matrix.yml (Line: 277, Col: 15): 
Error reading JToken from JsonReader. Path '', line 0, position 0.,.github/workflows/os-blas-test-matrix.yml (Line: 277, Col: 15): 
Unexpected value ''

???

@slivingston
Copy link
Member Author

@slivingston Despite the claim that all checks completed, in fact the conda-based tests did not complete. See OS/BLAS test matrix run. It looks like the conda test matrix gets created but does not run.

The error is

Error when evaluating 'strategy' for job 'test-conda'. 
.github/workflows/os-blas-test-matrix.yml (Line: 277, Col: 15): 
Error parsing fromJson,.github/workflows/os-blas-test-matrix.yml (Line: 277, Col: 15): 
Error reading JToken from JsonReader. Path '', line 0, position 0.,.github/workflows/os-blas-test-matrix.yml (Line: 277, Col: 15): 
Unexpected value ''

???

Good catch. Yes, I am investigating this and a similar change for Slycot on which I am working.

@slivingston slivingston marked this pull request as draft February 17, 2025 04:26
@slivingston
Copy link
Member Author

It looks like the same hidden error happened 2 weeks ago in a different PR: https://github.com/python-control/python-control/actions/runs/13107250760

@slivingston slivingston force-pushed the ci-add-defaults-miniforge branch from 8f33d82 to 9b228b1 Compare February 17, 2025 05:47
@slivingston slivingston marked this pull request as ready for review February 17, 2025 07:46
@slivingston
Copy link
Member Author

Commit 1ceeeea deleted required imports in a test matrix generation script, resulting in an uncaught exception. The backtrace from Python would then be used as the saved output in a step of the workflow file. One of the next jobs tries to parse this output as JSON, hence the error from JsonReader. This is treated as a syntax error in the workflow file, thus an incomplete rather than a failed job.

I modified the workflow files so that errors from the test matrix generation scripts surface clearly as failed jobs.

@slivingston slivingston force-pushed the ci-add-defaults-miniforge branch from 7d4a01f to baaae00 Compare February 17, 2025 08:03
@slivingston slivingston marked this pull request as draft February 17, 2025 08:03
@slivingston
Copy link
Member Author

...actually, still debugging. I will finish it in the morning.

@slivingston slivingston force-pushed the ci-add-defaults-miniforge branch from baaae00 to 04d729e Compare February 17, 2025 18:37
@slivingston slivingston force-pushed the ci-add-defaults-miniforge branch from 04d729e to 6c628f0 Compare February 17, 2025 19:03
@slivingston slivingston marked this pull request as ready for review February 17, 2025 19:36
@slivingston
Copy link
Member Author

This is ready for review.

@bnavigator
Copy link
Contributor

Sorry for the extra delete in 1ceeeea

@murrayrm murrayrm merged commit d11f05d into python-control:main Feb 17, 2025
68 checks passed
@slivingston slivingston mentioned this pull request Feb 17, 2025
Merged
@slivingston
Copy link
Member Author

Sorry for the extra delete

@bnavigator No worries! I only intended to find the origin, not accuse anyone.

@slivingston slivingston deleted the ci-add-defaults-miniforge branch February 17, 2025 23:18
@murrayrm murrayrm added this to the 0.10.2 milestone Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnavigator bnavigator bnavigator approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.10.2
Development

Successfully merging this pull request may close these issues.
4 participants
@slivingston @coveralls @murrayrm @bnavigator