Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#18927 closed enhancement (fixed)

Upgrade Cython to 0.23.1

Reported by: Jeroen Demeyer Owned by:
Priority: major Milestone: sage-6.9
Component: packages: standard Keywords:
Cc: Robert Bradshaw, François Bissey, Michele Borassi Merged in:
Authors: Jeroen Demeyer Reviewers: François Bissey
Report Upstream: N/A Work issues:
Branch: b6e57c4 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Volker Braun)

Upstream: http://cython.org/release/Cython-0.23.1.tar.gz


Features added

  • Support for coverage.py 4.0+ can be enabled by adding the plugin "Cython.Coverage" to the ".coveragerc" config file.
  • Annotated HTML source pages can integrate (XML) coverage reports.
  • Tracing is supported in nogil functions/sections and module init code.
  • When generators are used in a Cython module and the module imports the modules "inspect" and/or "asyncio", Cython enables interoperability by patching these modules during the import to recognise Cython's internal generator and coroutine types. This can be disabled by C compiling the module with "-D CYTHON_PATCH_ASYNCIO=0" or "-D CYTHON_PATCH_INSPECT=0"
  • When generators or coroutines are used in a Cython module, their types are registered with the Generator and Coroutine ABCs in the collections or collections.abc stdlib module at import time to enable interoperability with code that needs to detect and process Python generators/coroutines. These ABCs were added in CPython 3.5 and are available for older Python versions through the backports_abc module on PyPI. See https://bugs.python.org/issue24018
  • Adding/subtracting/dividing/modulus and equality comparisons with constant Python floats and small integers are faster.
  • Binary and/or/xor/rshift operations with small constant Python integers are faster.
  • When called on generator expressions, the builtins all(), any(), dict(), list(), set(), sorted() and unicode.join() avoid the generator iteration overhead by inlining a part of their functionality into the for-loop.
  • Keyword argument dicts are no longer copied on function entry when they are not being used or only passed through to other function calls (e.g. in wrapper functions).
  • wraparound() and boundscheck() are available as no-ops in pure Python mode.
  • Const iterators were added to the provided C++ STL declarations.
  • NULL is allowed as default argument when embedding signatures. This fixes ticket 843.
  • When compiling with --embed, the internal module name is changed to main to allow arbitrary program names, including those that would be invalid for modules. Note that this prevents reuse of the generated C code as an importable module.
  • External C++ classes that overload the assignment operator can be used. Patch by Ian Henriksen.

Bugs fixed

  • Language level 3 did not enable true division (a.k.a. float division) for integer operands.
  • Functions with fused argument types that included a generic 'object' fallback could end up using that fallback also for other explicitly listed object types.
  • Relative cimports could accidentally fall back to trying an absolute cimport on failure.
  • The result of calling a C struct constructor no longer requires an intermediate assignment when coercing to a Python dict.
  • C++ exception declarations with mapping functions could fail to compile when pre-declared in .pxd files.
  • cpdef void methods are now permitted.
  • abs(cint) could fail to compile in MSVC and used sub-optimal code in C++. Patch by David Vierra, original patch by Michael Enßlin.
  • Buffer index calculations using index variables with small C integer types could overflow for large buffer sizes. Original patch by David Vierra.
  • C unions use a saner way to coerce from and to Python dicts.
  • When compiling a module foo.pyx, the directories in sys.path are no longer searched when looking for foo.pxd. Patch by Jeroen Demeyer.
  • Memory leaks in the embedding main function were fixed. Original patch by Michael Enßlin.
  • Some complex Python expressions could fail to compile inside of finally clauses.

Other changes

  • Changed mangling scheme in header files generated by cdef api declarations.

Change History (31)

comment:1 Changed 7 years ago by Jeroen Demeyer

Description: modified (diff)

comment:2 Changed 7 years ago by Jeroen Demeyer

Branch: u/jdemeyer/upgrade_cython_to_0_23

comment:3 Changed 7 years ago by Jeroen Demeyer

Commit: 8fb35d747ae62e9655c832d79df2584b342e70bc
Description: modified (diff)

comment:4 Changed 7 years ago by François Bissey

Cc: François Bissey added

comment:5 Changed 7 years ago by Jeroen Demeyer

Description: modified (diff)

comment:6 Changed 7 years ago by git

Commit: 8fb35d747ae62e9655c832d79df2584b342e70bc4d35d080f69f6c702f7fba5de19a201e984d5858

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

4d35d08Upgrade to Cython 0.23 + add testsuite [work in progress]

comment:7 Changed 7 years ago by François Bissey

And it is now officially released. I am ready to put this to positive review once we have a tarball.

comment:8 Changed 7 years ago by Jeroen Demeyer

Description: modified (diff)
Milestone: sage-6.8sage-6.9

comment:9 Changed 7 years ago by git

Commit: 4d35d080f69f6c702f7fba5de19a201e984d58581185b2c839d3bb06b35a7b7f07061fb0fc6dde0f

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

1185b2cUpgrade to Cython 0.23 + add testsuite

comment:10 Changed 7 years ago by Jeroen Demeyer

Status: newneeds_review

comment:11 Changed 7 years ago by François Bissey

Reviewers: François Bissey
Status: needs_reviewpositive_review

comment:12 Changed 7 years ago by Volker Braun

Branch: u/jdemeyer/upgrade_cython_to_0_231185b2c839d3bb06b35a7b7f07061fb0fc6dde0f
Resolution: fixed
Status: positive_reviewclosed

comment:13 Changed 7 years ago by Volker Braun

Commit: 1185b2c839d3bb06b35a7b7f07061fb0fc6dde0f

The testsuite fails on 32-bit for the record: http://trac.cython.org/ticket/857

comment:14 Changed 7 years ago by Volker Braun

Resolution: fixed
Status: closednew

Reopening as it fails on some buildbots with error: reference to ‘uint64_t’ is ambiguous. I asked at https://groups.google.com/d/msg/cython-users/nOaoFjt8BLo/RI3vtsOMDwAJ for advice.

comment:15 Changed 7 years ago by Volker Braun

Branch: 1185b2c839d3bb06b35a7b7f07061fb0fc6dde0fu/jdemeyer/upgrade_cython_to_0_23
Commit: 1185b2c839d3bb06b35a7b7f07061fb0fc6dde0f

New commits:

1185b2cUpgrade to Cython 0.23 + add testsuite

comment:16 Changed 7 years ago by Volker Braun

Branch: u/jdemeyer/upgrade_cython_to_0_23u/vbraun/upgrade_cython_to_0_23

comment:17 Changed 7 years ago by Volker Braun

Cc: Michele Borassi added
Commit: 1185b2c839d3bb06b35a7b7f07061fb0fc6dde0f6608075748469195a6d63dcae5651db6cb8ec9f4
Status: newneeds_review

Regardless of how the cython list answers we should get rid of the global using namespace statements.


New commits:

6608075global "using namespace foo" statements are bad practice

comment:18 Changed 7 years ago by François Bissey

I think it is a good idea but I cannot check the impact on a 32bit platform.

comment:19 Changed 7 years ago by Volker Braun

This shouldn't have any impact on 32-bit platforms, it just avoids kneecapping the compilation unit from the get-go.

comment:20 Changed 7 years ago by Volker Braun

Description: modified (diff)
Summary: Upgrade Cython to 0.23Upgrade Cython to 0.23.1

comment:21 Changed 7 years ago by git

Commit: 6608075748469195a6d63dcae5651db6cb8ec9f4420cdf22f3c34dee0fbf03d96aecf4ac28f84faa

Branch pushed to git repo; I updated commit sha1. New commits:

420cdf2Update to Cython-0.23.1

comment:22 Changed 7 years ago by Volker Braun

Can somebody click on positive review?

comment:23 Changed 7 years ago by François Bissey

Status: needs_reviewpositive_review

There.

comment:24 Changed 7 years ago by git

Commit: 420cdf22f3c34dee0fbf03d96aecf4ac28f84faab6e57c46b714b1eb811a3a62fccfc37b6b5ff2ff
Status: positive_reviewneeds_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

8d8504cMerge Sage-6.9.beta3 into t/18927/upgrade_cython_to_0_23
b6e57c4more boost interface namespace fixes

comment:25 Changed 7 years ago by Volker Braun

Status: needs_reviewpositive_review

In the meantime more stuff without namespace appeared...

comment:26 Changed 7 years ago by François Bissey

Marginally related in a way. Should we have a separate audit of namespace and include a bit about them in the review section of the documentation?

comment:27 Changed 7 years ago by Volker Braun

Really we should put C++ Sage stuff into a sage namespace, then its safe to pull symbols in via using declarations

namespace sage {
    namespace graph {
        using std::vector;
        using boost:make_iterator_property_map;
        [...]
    }
}

comment:28 Changed 7 years ago by François Bissey

hum... Definitely need the namespace(s), that's just asking to be clobbered.

comment:29 Changed 7 years ago by Michele Borassi

Hello!

Thank you very much for telling me that using namespace is bad practice, and for correcting my code in boost_interface.cpp.

However, I have used vectors also in Cython, in files boost_graph.pyx and boost_graph.pxd, and I haven't used namespaces (for instance, by writing vector and not std::vector). Is it fine? Otherwise, what is the correct way to do it?

Best,

Michele

comment:30 Changed 7 years ago by Volker Braun

Branch: u/vbraun/upgrade_cython_to_0_23b6e57c46b714b1eb811a3a62fccfc37b6b5ff2ff
Resolution: fixed
Status: positive_reviewclosed

comment:31 Changed 7 years ago by Volker Braun

Commit: b6e57c46b714b1eb811a3a62fccfc37b6b5ff2ff

In Python its ok to import into the module namespace "from libcpp.vector cimport vector". Since there are no #include statements this will not cause conflicts in third-party headers. Its only in C++ that

using namespace foo;
#include <bar>

will then make bar see all of foo's symbols (and potentially trip over some).

Actuall in Cython you have pxi files which behave like #include, but you shouldn't use them if you can avoid it; they are mostly for historical reasons.

Note: See TracTickets for help on using tickets.