Skip to content

Conversation

@gwgundersen
Copy link
Contributor

@gwgundersen gwgundersen commented Aug 19, 2019

@gwgundersen
Copy link
Contributor Author

Strange. This passes all my tests locally.

@max-sixty
Copy link
Collaborator

The failure is b/c of an upstream package; we can ignore

@max-sixty max-sixty self-requested a review August 20, 2019 01:31
Copy link
Collaborator

@max-sixty max-sixty left a comment

Choose a reason for hiding this comment

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

Great, thanks for adding those two changes

I'll merge tomorrow unless any other changes

@gwgundersen
Copy link
Contributor Author

@shoyer, I've addressed your comments by adding more helpful warnings and testing that said warnings are raised properly. I've also moved both warnings to the top of the function as I think it's a bit easier to reason about the code branches this way.

@pep8speaks
Copy link

pep8speaks commented Aug 23, 2019

Hello @gwgundersen! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-08-23 20:57:22 UTC

@max-sixty max-sixty merged commit 487b087 into pydata:master Aug 23, 2019
@gwgundersen gwgundersen deleted the drop-keyword-amend branch August 23, 2019 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants