BUG: Fix complex to bool conversion in lowlevel_strided_loops #477

Merged
merged 2 commits into from Oct 4, 2012

Conversation

Projects
None yet
3 participants
@seberg
Member

seberg commented Oct 1, 2012

This is a fix for Ticket 2218. I am not sure if this is the best way to fix it, but I hunted it down to that place and just implemented the first fix that seemed reasonable.

On a side note, the arraytypes.c.src code seems to handle complex to bool conversion fine, but when does it get called at all?

@@ -1459,6 +1459,14 @@ def test_complex_scalar_complex_cast(self):
x = tp(1+2j)
assert_equal(complex(x), 1+2j)
+ def test_complex_boolean_cast(self):

This comment has been minimized.

Show comment Hide comment
@charris

charris Oct 2, 2012

Member

Might as well test the astype method too.

@charris

charris Oct 2, 2012

Member

Might as well test the astype method too.

This comment has been minimized.

Show comment Hide comment
@charris

charris Oct 2, 2012

Member

And maybe logical_or.reduce and logical_and.reduce as well.

In [1]: a = ones(3)*1j

In [2]: logical_or.reduce(a)
Out[2]: True

In [3]: logical_and.reduce(a)
Out[3]: True

You could maybe put those tests in numpy/core/test_ufunc. If you want to go for completeness, you could do it for all numeric types. The easy way to do that is to use the typecodes in np.typecodes

In [4]: np.typecodes
Out[4]: 
{'All': '?bhilqpBHILQPefdgFDGSUVOMm',
 'AllFloat': 'efdgFDG',
 'AllInteger': 'bBhHiIlLqQpP',
 'Character': 'c',
 'Complex': 'FDG',
 'Datetime': 'Mm',
 'Float': 'efdg',
 'Integer': 'bhilqp',
 'UnsignedInteger': 'BHILQP'}
@charris

charris Oct 2, 2012

Member

And maybe logical_or.reduce and logical_and.reduce as well.

In [1]: a = ones(3)*1j

In [2]: logical_or.reduce(a)
Out[2]: True

In [3]: logical_and.reduce(a)
Out[3]: True

You could maybe put those tests in numpy/core/test_ufunc. If you want to go for completeness, you could do it for all numeric types. The easy way to do that is to use the typecodes in np.typecodes

In [4]: np.typecodes
Out[4]: 
{'All': '?bhilqpBHILQPefdgFDGSUVOMm',
 'AllFloat': 'efdgFDG',
 'AllInteger': 'bBhHiIlLqQpP',
 'Character': 'c',
 'Complex': 'FDG',
 'Datetime': 'Mm',
 'Float': 'efdg',
 'Integer': 'bhilqp',
 'UnsignedInteger': 'BHILQP'}

This comment has been minimized.

Show comment Hide comment
@charris

charris Oct 2, 2012

Member

And if you do that and are working in the test directory you can run those tests by doing

charris@f16 [tests (pull-477)]$ python test_ufunc.py
@charris

charris Oct 2, 2012

Member

And if you do that and are working in the test directory you can run those tests by doing

charris@f16 [tests (pull-477)]$ python test_ufunc.py
@charris

This comment has been minimized.

Show comment Hide comment
@charris

charris Oct 2, 2012

Member

This looks good and works for me. If you make the tests more thorough that would be helpful in building up the numpy test coverage.

Member

charris commented Oct 2, 2012

This looks good and works for me. If you make the tests more thorough that would be helpful in building up the numpy test coverage.

@charris

This comment has been minimized.

Show comment Hide comment
@charris

charris Oct 4, 2012

Member

I'm going to put this in.. If you put together another PR with more extensive tests, I'll be glad to put that in also.

@certik I think this should go into 1.7.

Member

charris commented Oct 4, 2012

I'm going to put this in.. If you put together another PR with more extensive tests, I'll be glad to put that in also.

@certik I think this should go into 1.7.

@charris

This comment has been minimized.

Show comment Hide comment
@charris

charris Oct 4, 2012

Member

I don't see a separate compilation problem on my machine.

Member

charris commented Oct 4, 2012

I don't see a separate compilation problem on my machine.

charris added a commit that referenced this pull request Oct 4, 2012

Merge pull request #477 from seberg/ticket2218
BUG: Fix complex to bool conversion in lowlevel_strided_loops

@charris charris merged commit 3f10c36 into numpy:master Oct 4, 2012

1 check failed

default The Travis build failed
Details
@certik

This comment has been minimized.

Show comment Hide comment
@certik

certik Nov 14, 2012

Contributor

@charris, I just sent the PR #2741 backporting this.

Contributor

certik commented Nov 14, 2012

@charris, I just sent the PR #2741 backporting this.

certik added a commit that referenced this pull request Nov 16, 2012

@seberg seberg deleted the seberg:ticket2218 branch Dec 3, 2013

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