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

Use old Cython build_ext until includes are fixed. #14496

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
@robertwb
Contributor

robertwb commented Oct 26, 2016

This change in Cython 0.25

The distutils extension Cython.Distutils.build_ext has now been updated to use cythonize which properly handles dependencies. The old extension can still be found in Cython.Distutils.old_build_ext and is now deprecated.

seems to have broken the include of auto-generated algos_common_helper.pxi. As a temporary workaround, the old build_ext can be used.

Use old Cython build_ext until includes are fixed.
This change

The distutils extension Cython.Distutils.build_ext has now been updated to use cythonize which properly handles dependencies. The old extension can still be found in Cython.Distutils.old_build_ext and is now deprecated.

in Cython 0.25 seems to have broken includes. As a temporary workaround, the old build_ext can be used.
@robertwb

This comment has been minimized.

Show comment
Hide comment
@robertwb

robertwb Oct 26, 2016

Contributor

It looks like the difficulties are due to

  1. The new Cython.Distutils.build_ext runs cythonize() in finalize_options(), before the .pxi files are generated, and
  2. The extension objects use the include_dirs property, which is only inspected by the old build_ext.
Contributor

robertwb commented Oct 26, 2016

It looks like the difficulties are due to

  1. The new Cython.Distutils.build_ext runs cythonize() in finalize_options(), before the .pxi files are generated, and
  2. The extension objects use the include_dirs property, which is only inspected by the old build_ext.
@@ -85,7 +85,7 @@ def is_platform_mac():
try:
if not _CYTHON_INSTALLED:
raise ImportError('No supported version of Cython installed.')
from Cython.Distutils import build_ext as _build_ext

This comment has been minimized.

@jreback

jreback Oct 26, 2016

Contributor

well that will fail on older versions of cython wouldn't it?

need to check in a try except

further we use older (and newer versions) of cython in various builds

@jreback

jreback Oct 26, 2016

Contributor

well that will fail on older versions of cython wouldn't it?

need to check in a try except

further we use older (and newer versions) of cython in various builds

This comment has been minimized.

@robertwb

robertwb Oct 26, 2016

Contributor

Fixed.

@robertwb

robertwb Oct 26, 2016

Contributor

Fixed.

@jreback jreback added the Build label Oct 26, 2016

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Oct 26, 2016

Current coverage is 85.26% (diff: 100%)

Merging #14496 into master will decrease coverage by <.01%

@@             master     #14496   diff @@
==========================================
  Files           140        140          
  Lines         50667      50670     +3   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43203      43205     +2   
- Misses         7464       7465     +1   
  Partials          0          0          

Powered by Codecov. Last update d1d75d7...3d977c3

Current coverage is 85.26% (diff: 100%)

Merging #14496 into master will decrease coverage by <.01%

@@             master     #14496   diff @@
==========================================
  Files           140        140          
  Lines         50667      50670     +3   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43203      43205     +2   
- Misses         7464       7465     +1   
  Partials          0          0          

Powered by Codecov. Last update d1d75d7...3d977c3

@stonebig stonebig referenced this pull request Oct 26, 2016

Closed

release 2016-05 follow-up #381

@jreback

This comment has been minimized.

Show comment
Hide comment
@jreback

jreback Oct 26, 2016

Contributor

lgtm. can you add a release note, say compat with cython 0.25 builds.

was just about to say this isn't tested on travis, but cython JUST updated and broke our builds :>

Contributor

jreback commented Oct 26, 2016

lgtm. can you add a release note, say compat with cython 0.25 builds.

was just about to say this isn't tested on travis, but cython JUST updated and broke our builds :>

@jreback

This comment has been minimized.

Show comment
Hide comment
@jreback

jreback Oct 26, 2016

Contributor

so going to merge.

thanks!

Contributor

jreback commented Oct 26, 2016

so going to merge.

thanks!

@jreback jreback added this to the 0.19.1 milestone Oct 26, 2016

@jreback jreback closed this in 66b4c83 Oct 26, 2016

@jreback

This comment has been minimized.

Show comment
Hide comment
@jreback

jreback Oct 26, 2016

Contributor

I fixed the 3.4 build on the most recent cython. 6ac759d

If you would like to submit a PR to actually fix the way cython builds for >= 0.25 would be great! (need compat of course for older cythons). We still support >= 0.19.1, but should prob move that up.

Contributor

jreback commented Oct 26, 2016

I fixed the 3.4 build on the most recent cython. 6ac759d

If you would like to submit a PR to actually fix the way cython builds for >= 0.25 would be great! (need compat of course for older cythons). We still support >= 0.19.1, but should prob move that up.

jorisvandenbossche added a commit to jorisvandenbossche/pandas that referenced this pull request Nov 2, 2016

[Backport #14496] BLD: Support Cython 0.25
closes #14496

(cherry picked from commit 66b4c83)

yarikoptic added a commit to neurodebian/pandas that referenced this pull request Nov 18, 2016

Merge tag 'v0.19.1' into debian
Version 0.19.1

* tag 'v0.19.1': (43 commits)
  RLS: v0.19.1
  DOC: update whatsnew/release notes for 0.19.1 (#14573)
  [Backport #14545] BUG/API: Index.append with mixed object/Categorical indices (#14545)
  DOC: rst fixes
  [Backport #14567] DEPR: add deprecation warning for com.array_equivalent (#14567)
  [Backport #14551] PERF: casting loc to labels dtype before searchsorted (#14551)
  [Backport #14536] BUG: DataFrame.quantile with NaNs (GH14357) (#14536)
  [Backport #14520] BUG: don't close user-provided file handles in C parser (GH14418) (#14520)
  [Backport #14392] BUG: Dataframe constructor when given dict with None value (#14392)
  [Backport #14514] BUG: Don't parse inline quotes in skipped lines (#14514)
  [Bacport #14543] BUG: tseries ceil doc fix (#14543)
  [Backport #14541] DOC: Simplify the gbq integration testing procedure for contributors (#14541)
  [Backport #14527] BUG/ERR: raise correct error when sql driver is not installed (#14527)
  [Backport #14501] BUG: fix DatetimeIndex._maybe_cast_slice_bound for empty index (GH14354) (#14501)
  [Backport #14442] DOC: Expand on reference docs for read_json() (#14442)
  BLD: fix 3.4 build for cython to 0.24.1
  [Backport #14492] BUG: Accept unicode quotechars again in pd.read_csv
  [Backport #14496] BLD: Support Cython 0.25
  [Backport #14498] COMPAT/TST: fix test for range testing of negative integers to neg powers
  [Backport #14476] PERF: performance regression in Series.asof (#14476)
  ...

amolkahat added a commit to amolkahat/pandas that referenced this pull request Nov 26, 2016

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