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

Switch to building cython extensions at installation time #336

Merged
merged 3 commits into from
Mar 10, 2021

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Mar 8, 2021

We currently provide the C/C++ rendered files from Cython in our sdist and in the repository. This causes issues when Python changes its API and Cython needs to re-render the files to match. This PR changes the necessary pieces of the setup.py and CI so this rendering always happens on installation. Users without compilers will need to depend on the wheels on PyPI.

  • Tests added
  • Tests passed
  • Passes git diff origin/master **/*py | flake8 --diff
  • Fully documented

@djhoese djhoese requested a review from mraspaud March 8, 2021 21:12
@djhoese djhoese self-assigned this Mar 8, 2021
@codecov
Copy link

codecov bot commented Mar 8, 2021

Codecov Report

Merging #336 (606bfed) into master (49a3d7d) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #336   +/-   ##
=======================================
  Coverage   92.59%   92.59%           
=======================================
  Files          48       48           
  Lines        9847     9847           
=======================================
  Hits         9118     9118           
  Misses        729      729           
Flag Coverage Δ
unittests 92.59% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out 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 49a3d7d...1527a05. Read the comment docs.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.587% when pulling 1527a05 on djhoese:build-require-cython into 49a3d7d on pytroll:master.

Copy link
Member

@pnuu pnuu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. This will clean-up also the diffs a lot for the following PRs touching the Cython code.

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@mraspaud mraspaud merged commit 91ba39f into pytroll:master Mar 10, 2021
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.

None yet

4 participants