Skip to content

FIX: Correctly cast nan when testing array_to_file- fixes ARM64 builds #862

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

Merged
merged 4 commits into from
May 27, 2020

Conversation

effigies
Copy link
Member

@effigies effigies commented Jan 12, 2020

Trying to replicate #861.

Closes #861.

@codecov
Copy link

codecov bot commented Jan 12, 2020

Codecov Report

Merging #862 into maint/3.1.x will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           maint/3.1.x     #862      +/-   ##
===============================================
- Coverage        91.81%   91.77%   -0.05%     
===============================================
  Files               97       97              
  Lines            12359    12359              
  Branches          2177     2177              
===============================================
- Hits             11348    11343       -5     
- Misses             678      681       +3     
- Partials           333      335       +2     
Impacted Files Coverage Δ
nibabel/environment.py 75.00% <0.00%> (-20.00%) ⬇️
nibabel/casting.py 85.28% <0.00%> (-0.87%) ⬇️
nibabel/dft.py 80.36% <0.00%> (-0.62%) ⬇️
nibabel/nifti1.py 92.12% <0.00%> (+0.15%) ⬆️
nibabel/volumeutils.py 84.52% <0.00%> (+0.38%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 97d5d03...68579bf. Read the comment docs.

@effigies
Copy link
Member Author

effigies commented Jan 12, 2020

According to https://docs.travis-ci.com/user/multi-cpu-architectures/#testing-on-multiple-cpu-architectures, we can also check on ppc64le and s390x. Will look into those if this works smoothly.

Also, there's OSX support, so we should at least test on that.

@effigies
Copy link
Member Author

effigies commented Jan 12, 2020

@matthew-brett The ARM64 jobs require building numpy from source every time, and we don't currently have the necessary C dependencies installed. Would it be reasonable to open a PR against numpy-wheels to add non-x86 wheels to their .travis.yml? Would anything else be required apart from updating multibuild?

@kaczmarj
Copy link
Contributor

can you use miniconda? there are arm64 and powerpc miniconda installations, and numpy is compiled for arm64 and powerpc (https://anaconda.org/conda-forge/numpy).

@effigies
Copy link
Member Author

Numpy is working on wheels on their end. I don't think it's too urgent to wait on that.

@effigies effigies force-pushed the test/new_archs branch 2 times, most recently from dcd0bfc to d581590 Compare May 15, 2020 13:09
@effigies effigies mentioned this pull request May 15, 2020
@effigies effigies changed the base branch from master to maint/3.1.x May 24, 2020 20:23
@effigies effigies closed this May 24, 2020
@effigies effigies reopened this May 24, 2020
@effigies effigies changed the title CI: Add ARM64 build FIX: Correctly cast nan when testing array_to_file- fixes ARM64 builds May 24, 2020
@effigies
Copy link
Member Author

Anybody up for a review? Will merge Friday if nobody shouts.

@matthew-brett
Copy link
Member

Looks right to me - thanks for tracking that down.

@effigies
Copy link
Member Author

Great, thanks for having a look.

@effigies effigies merged commit 23de925 into nipy:maint/3.1.x May 27, 2020
@effigies effigies deleted the test/new_archs branch May 27, 2020 16:28
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.

Test failure on Arm64
3 participants