Skip to content

Conversation

@jakobjakobson13
Copy link
Contributor

@jakobjakobson13 jakobjakobson13 commented Aug 11, 2020

See #17012 item 16.

Closes #17076

@eric-wieser eric-wieser changed the title simplify "or" statement BUG: fix typo in polydiv that prevented promotion to poly1d Aug 11, 2020
@eric-wieser
Copy link
Member

It might be good to add a test for the return type of polydiv(some_arr, some_poly), which this patch changes.

@Dahlia-Chehata
Copy link

closes #17076

@eric-wieser
Copy link
Member

@Dahlia-Chehata: Not sure it was really worth opening an issue about that given we already have the patch, but no harm done.

@Dahlia-Chehata
Copy link

@eric-wieser you are right, but I discovered this PR after I already had opened the issue.

Comment on lines 250 to 251
assert_equal(isinstance(s, np.poly1d) == True)
assert_equal(isinstance(t, np.poly1d) == True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert_equal(isinstance(s, np.poly1d) == True)
assert_equal(isinstance(t, np.poly1d) == True)
assert isinstance(s, np.poly1d)
assert isinstance(t, np.poly1d)

Comment on lines 247 to 249
d = np.poly1d([1, 2, 3])
c = [1, 2, 3]
s, t = np.polydiv(d, c)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is backwards - its the one that passed before this patch.

Probably fine to keep it, but you should add one with the arguments in the other order too.

Copy link
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @jakobjakobson13

@rossbar rossbar merged commit 0dc5588 into numpy:master Aug 18, 2020
@jakobjakobson13 jakobjakobson13 deleted the cleanup_polynomial branch August 18, 2020 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: Typo in numpy.polydiv

4 participants