Skip to content

#Issue272 Cythonvscython#274

Merged
Didou09 merged 10 commits intodevelfrom
Issue272_Cythonvscython
Nov 22, 2019
Merged

#Issue272 Cythonvscython#274
Didou09 merged 10 commits intodevelfrom
Issue272_Cythonvscython

Conversation

@Didou09
Copy link
Copy Markdown
Member

@Didou09 Didou09 commented Nov 21, 2019

Fixes, in devel, issue #272 to make pip install easier

@Didou09 Didou09 requested a review from lasofivec November 21, 2019 17:04
@Didou09 Didou09 self-assigned this Nov 21, 2019
@pep8speaks
Copy link
Copy Markdown

pep8speaks commented Nov 21, 2019

Hello @Didou09! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 31:5: E301 expected 1 blank line, found 0
Line 34:5: E301 expected 1 blank line, found 0
Line 52:1: E402 module level import not at top of file

Comment last updated at 2019-11-22 15:19:38 UTC

@codecov-io
Copy link
Copy Markdown

codecov-io commented Nov 21, 2019

Codecov Report

Merging #274 into devel will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##            devel     #274   +/-   ##
=======================================
  Coverage   41.46%   41.46%           
=======================================
  Files          79       79           
  Lines       23228    23228           
=======================================
  Hits         9632     9632           
  Misses      13596    13596
Impacted Files Coverage Δ
tofu/version.py 100% <100%> (ø) ⬆️

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 7a419d2...d835576. Read the comment docs.

@lasofivec
Copy link
Copy Markdown
Collaborator

I'm not sure this would work...
I opened a new branch yesterday

@lasofivec
Copy link
Copy Markdown
Collaborator

I wouldn't change the Cython/cython if they were not the issue in the first place... what do you think @Didou09

…xcept at import time also for np.get_include()
@lasofivec
Copy link
Copy Markdown
Collaborator

@Didou09
Copy link
Copy Markdown
Member Author

Didou09 commented Nov 22, 2019

Fall back to a more generic and modern solution as recommended in setuptools's answer to the opened issue (see few last comments on Issue #272 )

Closing the PR as we are changing solution

@Didou09 Didou09 closed this Nov 22, 2019
…tions showed that there exist a recommended solution from PEP517 and PEP518: introduce a tofu.toml file for listing build-time dependencies. Opening issue #276 accordingly
@Didou09
Copy link
Copy Markdown
Member Author

Didou09 commented Nov 22, 2019

Sorry, should have merged this before closing, there are still interesting changes (try / except + setup_requires) that can be useful.

@Didou09 Didou09 reopened this Nov 22, 2019
@Didou09
Copy link
Copy Markdown
Member Author

Didou09 commented Nov 22, 2019

To be clairifie din teh future branch @lasofivec : are the changes implemented here (try / except func + setup_requires) still useful / necessary even with the toml file ?

@Didou09 Didou09 merged commit ccaeae2 into devel Nov 22, 2019
@Didou09 Didou09 deleted the Issue272_Cythonvscython branch November 22, 2019 15:53
@Didou09 Didou09 mentioned this pull request Jan 30, 2020
@Didou09 Didou09 mentioned this pull request Mar 10, 2020
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.

4 participants