Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(466)

Issue 1105: `piecewise` exposes raw memory (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 11 months ago by stefanv
Modified:
4 years, 8 months ago
Base URL:
http://svn.scipy.org/svn/numpy/trunk/
Visibility:
Public.

Description

Patch Set 1 #

Patch Set 2 : Spaced as suggested in PEP-7. #

Patch Set 3 : Use `tuple` instead of types.TupleType. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+48 lines, -5 lines) Patch
numpy/lib/function_base.py View 1 2 4 chunks +8 lines, -5 lines 0 comments Download
numpy/lib/tests/test_function_base.py View 1 2 1 chunk +40 lines, -0 lines 0 comments Download

Messages

Total messages: 5
ondrej.certik
The patch looks good to me, I only have a couple of minor comments. http://codereview.appspot.com/1105/diff/1/3 ...
5 years, 11 months ago #1
stefanv
http://codereview.appspot.com/1105/diff/1/3 File numpy/lib/function_base.py (right): http://codereview.appspot.com/1105/diff/1/3#newcode569 Line 569: condlist = [asarray(c,dtype=bool) for c in condlist] On ...
5 years, 11 months ago #2
ondrej.certik
Now the patch looks good to me.
5 years, 11 months ago #3
Robert Kern
http://codereview.appspot.com/1105/diff/22/122 File numpy/lib/function_base.py (right): http://codereview.appspot.com/1105/diff/22/122#newcode566 Line 566: if not (isinstance(condlist[0], types.ListType) or types.ListType is unnecessary. ...
5 years, 11 months ago #4
stefanv
5 years, 11 months ago #5
Updated to use `tuple` instead of `types.TupleType`.  Also fixed some other
occurrences, like `types.StringType`.

http://codereview.appspot.com/1105/diff/22/122
File numpy/lib/function_base.py (right):

http://codereview.appspot.com/1105/diff/22/122#newcode566
Line 566: if not (isinstance(condlist[0], types.ListType) or
On 2008/05/21 20:10:28, Robert Kern wrote:
> types.ListType is unnecessary. list itself is the type.

Well spotted, thank you.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1278:e6ce13d99bf5