Ticket #3530 (closed enhancement: fixed)

Opened 5 years ago

Last modified 5 years ago

[with patch, with positive review] documentation for clib, cinclude pragmas

Reported by: malb Owned by: malb
Priority: minor Milestone: sage-3.0.4
Component: misc Keywords: cython, documentation
Cc: craigcitro, wstein Work issues:
Report Upstream: Reviewers:
Authors: Merged in:
Dependencies: Stopgaps:

Description

Craig wrote off list:

hey martin -- william tells me you created these pragmas for .spyx files. can you document them somewhere?

This patch documents the pragmas in the docstring. I also changed the behaviour of atlas() which now assumes that ATLAS is installed, since we ship it. mabshoff/wstein please check if my assumption is correct e.g. on OSX.

Attachments

clib_pragmas.patch Download (3.6 KB) - added by malb 5 years ago.
trac_3530_clib_pragmas-doctest-fixes.patch Download (2.1 KB) - added by mabshoff 5 years ago.
Oops, somebody forgot to doctest on his install :)

Change History

Changed 5 years ago by malb

comment:1 Changed 5 years ago by malb

Craig can you review my patch?

comment:2 Changed 5 years ago by craigcitro

  • Summary changed from [with patch, needs review] documentation for clib, cinclude pragmas to [with patch, under review by craigcitro before 7/5] documentation for clib, cinclude pragmas

I'm on it.

comment:3 Changed 5 years ago by craigcitro

  • Summary changed from [with patch, under review by craigcitro before 7/5] documentation for clib, cinclude pragmas to [with patch, with positive review pending one question] documentation for clib, cinclude pragmas

This looks great. I've already updated the note on wiki.sagemath.org to include a reference to this docstring for more info.

I also don't know the answer to Martin's question about ATLAS -- mabshoff or wstein, do you want to answer that?

comment:4 Changed 5 years ago by mabshoff

The patch will break some functionality since SAGE_CBLAS is no longer checked. I also tend to think that on OSX the whole BLAS function has not worked and will not ever work as is, so I would recommend splitting that part off to another ticket and dealing with it there. I can merge the "good" part and open another ticket for the "bad" part of the patch.

Cheers,

Michael

comment:5 Changed 5 years ago by malb

I'm all for splitting, so please go ahead. Actually, I don't think that this patch breaks anything it just reveals something that was broken for some time now.

comment:6 Changed 5 years ago by mabshoff

  • Summary changed from [with patch, with positive review pending one question] documentation for clib, cinclude pragmas to [with patch, with positive review] documentation for clib, cinclude pragmas

Since I agree with malb I will merge the patch as is and have opened #3563 to deal with the issue on OSX.

Cheers,

Michael

comment:7 Changed 5 years ago by mabshoff

  • Status changed from new to closed
  • Resolution set to fixed

Merged in Sage 3.0.4.alpha2

Changed 5 years ago by mabshoff

Oops, somebody forgot to doctest on his install :)

comment:8 Changed 5 years ago by craigcitro

Looks good. *sheepishly*: I actually doctested it this time. :)

Note: See TracTickets for help on using tickets.