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

TEST: Bring back Python 3.8 testing to GHA workflows #2954

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

jhlegarreta
Copy link
Contributor

@jhlegarreta jhlegarreta commented Oct 24, 2023

Bring back Python 3.8 testing to GHA workflows.

Removed in commit cf4338d.

Raise the minimum required NumPy version for the Python 3.8
compatibility test to 1.23.0 from the previously required 1.22.0 version
in commit 318bfc6 so that tests can pass. Fixes:

ImportError: numpy.core.multiarray failed to import

raised for example in:
https://github.com/dipy/dipy/actions/runs/6620737573/job/17983666930#step:9:12437

@codecov
Copy link

codecov bot commented Oct 24, 2023

Codecov Report

Merging #2954 (6c3a526) into master (3ad9ec1) will increase coverage by 0.01%.
Report is 109 commits behind head on master.
The diff coverage is 80.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2954      +/-   ##
==========================================
+ Coverage   81.77%   81.79%   +0.01%     
==========================================
  Files         146      146              
  Lines       20401    20401              
  Branches     3238     3238              
==========================================
+ Hits        16682    16686       +4     
+ Misses       2901     2898       -3     
+ Partials      818      817       -1     
Files Coverage Δ
dipy/data/fetcher.py 40.25% <ø> (ø)
dipy/info.py 100.00% <100.00%> (ø)
dipy/io/stateful_tractogram.py 76.96% <100.00%> (ø)
dipy/reconst/dti.py 92.57% <0.00%> (ø)
dipy/tracking/local_tracking.py 94.44% <96.29%> (ø)
dipy/io/utils.py 88.83% <90.00%> (ø)
dipy/io/streamline.py 84.92% <75.00%> (ø)
dipy/align/imwarp.py 95.60% <37.50%> (ø)

... and 1 file with indirect coverage changes

@skoudoro
Copy link
Member

Hi @jhlegarreta,

the minimal py38 is failing due to numpy version. Can you update it ?

@jhlegarreta
Copy link
Contributor Author

the minimal py38 is failing due to numpy version. Can you update it ?

I've seen it, Serge. Allow me some time, please.

Bring back Python 3.8 testing to GHA workflows.

Removed in commit cf4338d.

Raise the minimum required NumPy version for the Python 3.8
compatibility test to 1.23.0 from the previously required 1.22.0 version
in commit 318bfc6 so that tests can pass. Fixes:
```
ImportError: numpy.core.multiarray failed to import
```

raised for example in:
https://github.com/dipy/dipy/actions/runs/6620737573/job/17983666930#step:9:12437
@jhlegarreta
Copy link
Contributor Author

I've seen it, Serge. Allow me some time, please.

Done.

Codecov failure is unrelated.

Copy link
Contributor

@arokem arokem left a comment

Choose a reason for hiding this comment

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

LGTM!

@jhlegarreta
Copy link
Contributor Author

BTW, tried two times (1.22.0 : 1.22.4 : 1.23.0) to raise the min required numpy until tests passed with 1.23.0, this version being greater than the ones required for Python 3.9 and 3.10 (1.22.4)

I'd guess that this numpy incompatibility issue may be related to the circular dependency between trx and DIPY. I agree that it should probably happen with other Python versions as well; not sure it only happens with 3.8. Fixing the circular dependency seems a good step going forward.

@skoudoro
Copy link
Member

BTW, tried two times (1.22.0 : 1.22.4 : 1.23.0) to raise the min required numpy until tests passed with 1.23.0, this version being greater than the ones required for Python 3.9 and 3.10 (1.22.4)

I'd guess that this numpy incompatibility issue may be related to the circular dependency between trx and DIPY. I agree that it should probably happen with other Python versions as well; not sure it only happens with 3.8. Fixing the circular dependency seems a good step going forward.

I will tag @frheault, to look at this topic.

For now, thank you @jhlegarreta. merging

@skoudoro skoudoro merged commit 3ffac53 into dipy:master Oct 26, 2023
27 of 28 checks passed
@jhlegarreta jhlegarreta deleted the BringBackPython38ToGHATesting branch October 26, 2023 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants