Opened 10 years ago

Closed 10 years ago

#12975 closed defect (fixed)

Fix misleading typo in the doc of "cython"

Reported by: SimonKing Owned by: mvngu
Priority: trivial Milestone: sage-5.1
Component: interfaces Keywords:
Cc: burcin Merged in: sage-5.1.beta1
Authors: Burcin Erocal Reviewers: Simon King
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by SimonKing)

sage: cython?
       For example:
          #clang C++
          #clib givaro
          #cinclude /usr/local/include/
          #cargs -ggdb
          #cfile foo.c

That's misleading, as Cython does only accept #clang c++ but not #clang C++. Thank you, Burcin, for pointing that out!


Attachments (2)

trac_12975-cython_cpp_pragma.patch (573 bytes) - added by burcin 10 years ago.
trac_12975_reviewer.patch (2.1 KB) - added by SimonKing 10 years ago.
doc test for Burcin's patch

Download all attachments as: .zip

Change History (11)

comment:1 Changed 10 years ago by SimonKing

  • Cc burcin added
  • Status changed from new to needs_review

C++ typo fixed.

comment:2 Changed 10 years ago by burcin

I attached a new patch which should allow cython() to accept #clang C++ as well as #clang c++. I have no clue how to doctest this.

comment:3 Changed 10 years ago by SimonKing

With your patch, the following still does not work

sage: cython("""#clang C++
....: #cinclude /home/simon/SAGE/sage-5.0/local/include/singular /home/simon/SAGE/sage-5.0/local/include/factory 
....: #clib m readline singular givaro ntl gmpxx gmp
....: from sage.rings.polynomial.multi_polynomial_libsingular cimport MPolynomial_libsingular
....: """)
RuntimeError                              Traceback (most recent call last)

/home/simon/SAGE/sage-5.0/devel/sage-main/<ipython console> in <module>()

/home/simon/SAGE/sage-5.0/local/lib/python2.7/site-packages/sage/misc/ in sage.misc.cython_c.cython (sage/misc/cython_c.c:688)()

/home/simon/SAGE/sage-5.0/local/lib/python2.7/site-packages/sage/server/support.pyc in cython_import_all(filename, globals, verbose, compile_message, use_cache, create_local_c_file)
    471     m = cython_import(filename, verbose=verbose, compile_message=compile_message,
    472                      use_cache=use_cache,
--> 473                      create_local_c_file=create_local_c_file)
    474     for k, x in m.__dict__.iteritems():
    475         if k[0] != '_':

/home/simon/SAGE/sage-5.0/local/lib/python2.7/site-packages/sage/server/support.pyc in cython_import(filename, verbose, compile_message, use_cache, create_local_c_file, **kwds)
    448                                             use_cache=use_cache,
    449                                             create_local_c_file=create_local_c_file,
--> 450                                             **kwds)
    451     sys.path.append(build_dir)
    452     return __builtin__.__import__(name)

/home/simon/SAGE/sage-5.0/local/lib/python2.7/site-packages/sage/misc/cython.pyc in cython(filename, verbose, compile_message, use_cache, create_local_c_file, annotate, sage_namespace, create_local_so_file)
    529         log = open('%s/log'%build_dir).read()
    530         err = open('%s/err'%build_dir).read()
--> 531         raise RuntimeError, "Error compiling %s:\n%s\n%s"%(filename, log, err)
    533     # Move from lib directory.

RuntimeError: Error compiling /home/simon/.sage//temp/
running build
running build_ext
building '_home_simon__sage_temp_linux_sqwp_site_5382_tmp_2_spyx_0' extension
creating build
creating build/temp.linux-x86_64-2.7
gcc -fno-strict-aliasing -fwrapv -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -fPIC -I/home/simon/SAGE/sage-5.0/local/include/singular -I/home/simon/SAGE/sage-5.0/local/include/factory -I/home/simon/SAGE/sage-5.0/local/include/csage/ -I/home/simon/SAGE/sage-5.0/local/include/ -I/home/simon/SAGE/sage-5.0/local/include/python2.7/ -I/home/simon/SAGE/sage-5.0/local/lib/python2.7/site-packages/numpy/core/include -I/home/simon/SAGE/sage-5.0/devel/sage/sage/ext/ -I/home/simon/SAGE/sage-5.0/devel/sage/ -I/home/simon/SAGE/sage-5.0/devel/sage/sage/gsl/ -I/home/simon/.sage//temp/ -I/home/simon/SAGE/sage-5.0/local/include/python2.7 -c _home_simon__sage_temp_linux_sqwp_site_5382_tmp_2_spyx_0.cpp -o build/temp.linux-x86_64-2.7/_home_simon__sage_temp_linux_sqwp_site_5382_tmp_2_spyx_0.o -w -O2

gcc: error: _home_simon__sage_temp_linux_sqwp_site_5382_tmp_2_spyx_0.cpp: Datei oder Verzeichnis nicht gefunden
gcc: fatal error: no input files
compilation terminated.
error: command 'gcc' failed with exit status 1

Hence, one still needs

sage: cython("""#clang c++
#cinclude /home/simon/SAGE/sage-5.0/local/include/singular /home/simon/SAGE/sage-5.0/local/include/factory 
#clib m readline singular givaro ntl gmpxx gmp
from sage.rings.polynomial.multi_polynomial_libsingular cimport MPolynomial_libsingular

comment:4 Changed 10 years ago by SimonKing

My reviewer patch will contain a test such as this:


    Before :trac:`12975`, it would have beeen needed to write ``#clang c++``,
    but upper case ``C++`` has resulted in an error::

        sage: from sage.all import SAGE_ROOT
        sage: code = [                     
        ... "#clang C++",
        ... "#cinclude %s/local/include/singular %s/local/include/factory"%(SAGE_ROOT,SAGE_ROOT),
        ... "#clib m readline singular givaro ntl gmpxx gmp",
        ... "from sage.rings.polynomial.multi_polynomial_libsingular cimport MPolynomial_libsingular",
        ... "from sage.libs.singular.polynomial cimport singular_polynomial_pow",
        ... "def test(MPolynomial_libsingular p):",
        ... "    singular_polynomial_pow(&p._poly, p._poly, 2, p._parent_ring)"]
        sage: cython(os.linesep.join(code))

    The function ``test`` now manipulates internal C data of polynomials,
    squaring them::

        sage: P.<x,y>=QQ[]
        sage: test(x)
        sage: x

But with your current patch, one would still need c++, not C++.

Changed 10 years ago by burcin

comment:5 Changed 10 years ago by burcin

  • Authors changed from Simon King to Simon King, Burcin Erocal

New patch attached with same name. This one should do it. :)

Changed 10 years ago by SimonKing

doc test for Burcin's patch

comment:6 Changed 10 years ago by SimonKing

  • Authors changed from Simon King, Burcin Erocal to Burcin Erocal
  • Description modified (diff)
  • Reviewers set to Simon King

I have successfully run the tests in sage/misc with your patch and my reviewer patch. I will wait for a full test run (either by myself or the patchbot) before giving a positive review.

Apply trac_12975-cython_cpp_pragma.patch trac_12975_reviewer.patch

comment:7 Changed 10 years ago by SimonKing

  • Status changed from needs_review to positive_review

make ptest passes, and thus I give it a positive review.

comment:8 Changed 10 years ago by SimonKing

  • Component changed from documentation to interfaces

I changed the component into "interfaces" (to cython), since the fix was not just modifying the documentation, after all.

comment:9 Changed 10 years ago by jdemeyer

  • Merged in set to sage-5.1.beta1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.