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] Pre-build : From relative to absolute import #1741

Merged
merged 2 commits into from Mar 1, 2019

Conversation

@skoudoro
Copy link
Member

commented Feb 15, 2019

The goal of this PR is to fix Travis PRE build error. As you can see here, DIPY was failing during compilation due to the change of the default Cython language_level (from py2 to py3str, warning here). This implies to switch our relative import to absolute import to respect python 3 semantics.

So, this is just an update of some *.pyx and *.pxd files. Let's see if it works now...

@skoudoro

This comment has been minimized.

Copy link
Member Author

commented Feb 15, 2019

Hi @matthew-brett,

How can I set up the Cython directive language_level on our setup.py?

What is the goal of fake_pyrex folder? Do we still need this workaround?

Thank you for your feedback

@codecov-io

This comment has been minimized.

Copy link

commented Feb 15, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@04ddc4e). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1741   +/-   ##
=========================================
  Coverage          ?   84.26%           
=========================================
  Files             ?      115           
  Lines             ?    13608           
  Branches          ?     2145           
=========================================
  Hits              ?    11467           
  Misses            ?     1643           
  Partials          ?      498
@arokem

This comment has been minimized.

Copy link
Member

commented Feb 15, 2019

Looks like pure win to me. Unless anyone pipes up, I will merge this on Monday.

@skoudoro : do you want to move the comment #1741 (comment) to an issue for discussion?

@skoudoro

This comment has been minimized.

Copy link
Member Author

commented Feb 15, 2019

do you want to move the comment...

You can move it to an issue when you merge this PR on Monday. If I get an answer before Monday, I will try to update this PR.

@arokem

This comment has been minimized.

Copy link
Member

commented Feb 19, 2019

Hey @skoudoro : do you understand WTH is up with Appveyor? I thought we squashed that issue in #1730

@skoudoro

This comment has been minimized.

Copy link
Member Author

commented Feb 19, 2019

For sure, we did not squash that issue in #1730. I do not understand this random problem and it is hard to reproduce it on my environment, so hard to fix it.
I really believe that #1724, #1694, #1587 are all related to that random issue

@skoudoro

This comment has been minimized.

Copy link
Member Author

commented Mar 1, 2019

This PR is ready to go! Does anyone else want to take a look?

@arokem arokem merged commit 0c267a0 into nipy:master Mar 1, 2019

4 checks passed

codecov/patch Coverage not affected.
Details
codecov/project No report found to compare against
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@skoudoro skoudoro deleted the skoudoro:fix-absolute-pxd branch Mar 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.