massich / findmkl_openmeeg

0 stars 0 forks source link

[MRG] Add MatIO #8

Closed massich closed 6 years ago

massich commented 6 years ago
agramfort commented 6 years ago

you don't need all this code to test Matio

just put back MathsIO.C in OpenMEEGMaths_SOURCES

and that should it to test.

codecov[bot] commented 6 years ago

Codecov Report

Merging #8 into master will decrease coverage by 3.04%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master       #8      +/-   ##
==========================================
- Coverage   75.29%   72.25%   -3.05%     
==========================================
  Files          28       30       +2     
  Lines        1417     1665     +248     
==========================================
+ Hits         1067     1203     +136     
- Misses        350      462     +112
Impacted Files Coverage Δ
OpenMEEGMaths/src/MathsIO.C 43.37% <0%> (-15.67%) :arrow_down:
OpenMEEGMaths/include/Exceptions.H 0% <0%> (-15.52%) :arrow_down:
OpenMEEGMaths/include/vector.h 90% <0%> (-10%) :arrow_down:
OpenMEEGMaths/include/AsciiIO.H 88.33% <0%> (-5%) :arrow_down:
OpenMEEGMaths/src/vector.cpp 47.5% <0%> (-5%) :arrow_down:
OpenMEEGMaths/src/symmatrix.cpp 36.36% <0%> (-4.05%) :arrow_down:
OpenMEEGMaths/src/sparse_matrix.cpp 55.55% <0%> (-3.71%) :arrow_down:
OpenMEEGMaths/src/matrix.cpp 72.1% <0%> (-2.73%) :arrow_down:
OpenMEEGMaths/include/TrivialBinIO.H 90% <0%> (-0.91%) :arrow_down:
OpenMEEGMaths/tests/test_mat_files_io.cpp 69.23% <0%> (ø) :arrow_up:
... and 4 more

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 91cca6c...8930692. Read the comment docs.

agramfort commented 6 years ago

but should link statically with matio if we can to avoid shipping the so

massich commented 6 years ago

I think we should be able to build independently of the shipping policy

agramfort commented 6 years ago

I agree but then we need to make that we actually test if we use libmatio.a or libmatio.so

agramfort commented 6 years ago

ok green !

massich commented 6 years ago

@agramfort I think we can merge it as it is, and then take care of this error. Some dll is doing something werid, but I cannot see it in the dependency walk

massich commented 6 years ago

This PR got Merged with @agramfort +1