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

Flags correction for windows #1312

Merged
merged 2 commits into from Aug 4, 2017

Conversation

Projects
None yet
4 participants
@skoudoro
Member

skoudoro commented Jul 31, 2017

The goal is this PR is to setup correctly flags following the operating system. Moreover This should resolve #804.

skoudoro added some commits Jul 31, 2017

cython compiler flags correction: Defining flags following OS. We act…
…ivate openmp correctly and remove some unrecognized flags
@codecov-io

This comment has been minimized.

codecov-io commented Jul 31, 2017

Codecov Report

Merging #1312 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1312   +/-   ##
=======================================
  Coverage   87.09%   87.09%           
=======================================
  Files         228      228           
  Lines       28816    28816           
  Branches     3092     3092           
=======================================
  Hits        25098    25098           
  Misses       3014     3014           
  Partials      704      704

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 0c10a43...811d4b2. Read the comment docs.

@coveralls

This comment has been minimized.

coveralls commented Jul 31, 2017

Coverage Status

Coverage increased (+0.003%) to 85.408% when pulling 811d4b2 on skoudoro:openmp-windows into 0c10a43 on nipy:master.

1 similar comment
@coveralls

This comment has been minimized.

coveralls commented Jul 31, 2017

Coverage Status

Coverage increased (+0.003%) to 85.408% when pulling 811d4b2 on skoudoro:openmp-windows into 0c10a43 on nipy:master.

('dipy.align.parzenhist', [], 'c'),
('dipy.utils.omp', [], 'c')):
('dipy.reconst.peak_direction_getter', [], 'c'),

This comment has been minimized.

@Garyfallidis

Garyfallidis Aug 1, 2017

Member

too much indentation

[['-msse2', '-mfpmath=sse'], [], simple_test_c, 'USING_GCC_SSE2'],
[['-fopenmp'], ['-fopenmp'], omp_test_c, 'HAVE_OPENMP']], 'dipy')
msc_flag_defines = [[['/openmp'], [], omp_test_c, 'HAVE_VC_OPENMP'],

This comment has been minimized.

@Garyfallidis

Garyfallidis Aug 1, 2017

Member

I think msc supports SSE2

# Test if it is a 32 bits version
if not sys.maxsize > 2 ** 32:
# This flag is needed only on 32 bits
msc_flag_defines += [[['/arch:SSE2'], [], simple_test_c, 'USING_VC_SSE2'], ]

This comment has been minimized.

@Garyfallidis

Garyfallidis Aug 1, 2017

Member

Ah I see. It is here.

package_data = {'dipy':
[pjoin('data', 'files', '*')
]},
package_data={'dipy':

This comment has been minimized.

@Garyfallidis

Garyfallidis Aug 1, 2017

Member

Pep 8. Spaces around operators.

This comment has been minimized.

@Garyfallidis

Garyfallidis Aug 1, 2017

Member

Correct everywhere please.

This comment has been minimized.

@skoudoro

skoudoro Aug 1, 2017

Member

As you can see here, PEP 8 recommend not having spaces around = in a keyword argument or a default parameter value. I correct it everywhere for this reason. Are you sure it needs a revert ?

@Garyfallidis Garyfallidis merged commit a0df597 into nipy:master Aug 4, 2017

4 checks passed

codecov/patch Coverage not affected when comparing 0c10a43...811d4b2
Details
codecov/project 87.09% remains the same compared to 0c10a43
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.003%) to 85.408%
Details
@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Aug 4, 2017

OpenMP in Windows done! OSX is next :)

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Aug 4, 2017

Thanks @skoudoro!

@skoudoro skoudoro deleted the skoudoro:openmp-windows branch Aug 8, 2017

ShreyasFadnavis pushed a commit to ShreyasFadnavis/dipy that referenced this pull request Sep 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment