Skip to content
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.

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

Fix GitHub workflow CI #438

Merged
merged 3 commits into from
Dec 11, 2024
Merged

Fix GitHub workflow CI #438

merged 3 commits into from
Dec 11, 2024

Conversation

trundev
Copy link
Contributor

@trundev trundev commented Dec 8, 2024

Currently, CI fails because of following reasons:

  • Workflow job "lint" job, step "Run flake8" detects some code quality issues
  • Workflow uses python 3.5, which is no longer supported by GitHub Ubuntu runner
  • tools/rotor_estimation.py imports scipy.e, which is no longer present (see scipy/scipy@6e207a1)

This change also avoids some warnings about outdated GitHub actions versions.

Note:
Currently, the workflow is expected to pass for python 3.8 only, as it uses numba<0.59.
This is due to the @generated_jit issue and some other minor issues.

Issues detected by flake8:
4     E275 missing whitespace after keyword
1     E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`
6     F401 'random' imported but unused
5     F841 local variable 'biv2' is assigned to but never used
* scipy.e is deprecated, use numpy.e instead
  see scipy/scipy@6e207a1
* Migrate to the latest github actions
* Replace the unsupported python 3.5 with 3.8
* Use python 3.11 instead of 3.8, and 3.12 instead of 3.9
@@ -14,12 +14,12 @@ jobs:
strategy:
fail-fast: false
matrix:
python-version: [3.8]
python-version: [3.11]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
python-version: [3.11]
python-version: ["3.11"]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(and throughout; otherwise when we eventually reach 3.20, yaml will parse it as 3.2!)

Copy link
Contributor Author

@trundev trundev Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true, 3.20 is the same as 3.2, i.e. must use "3.20".
BTW, this is valid for "3.10" also - I skipped it, but it could soon become the "legacy" version.

So, quotes are mandatory for versions that ends in zeros, otherwise - recommended. I suggest quoting all the versions. Is this ok?

clifford/cga.py Outdated Show resolved Hide resolved
@eric-wieser eric-wieser marked this pull request as ready for review December 9, 2024 03:17
Remove leftovers from python 2 compatibility

Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Copy link

codecov bot commented Dec 9, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 67.28%. Comparing base (1102eba) to head (e362d3e).
Report is 14 commits behind head on master.

Files with missing lines Patch % Lines
clifford/cga.py 0.00% 1 Missing ⚠️
clifford/tools/g3c/rotor_estimation.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #438      +/-   ##
==========================================
- Coverage   71.35%   67.28%   -4.08%     
==========================================
  Files          73       74       +1     
  Lines        9461     9460       -1     
  Branches     1313     1212     -101     
==========================================
- Hits         6751     6365     -386     
- Misses       2539     2916     +377     
- Partials      171      179       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@arsenovic arsenovic merged commit e362d3e into pygae:master Dec 11, 2024
4 of 13 checks passed
@trundev trundev deleted the fix-github-ci branch December 11, 2024 14:56
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.

3 participants