Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix errors in the test suite due to pytest warning changes #1423

Merged
merged 3 commits into from
May 24, 2017

Conversation

shoyer
Copy link
Member

@shoyer shoyer commented May 24, 2017

This is causing CI failures for some builds.

This is really a pytest bug (pytest-dev/pytest#2430), but working around this on our end is easy enough (and cleans things up a little).

  • Passes git diff upstream/master | flake8 --diff

This is causing CI failures for some builds.

This is really a pytest bug (pytest-dev/pytest#2430),
but working around this on our end is easy enough (and cleans things up a
little).
@shoyer shoyer mentioned this pull request May 24, 2017
4 tasks
yield
assert len(w) > 0
assert any(message in str(wi.message) for wi in w)
assert len(w) > 0
Copy link
Contributor

Choose a reason for hiding this comment

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

is the change in indentation intentional? I would expect w to leave scope.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this was intentional. Python with blocks don't define scopes for variables, so w is still defined outside the block. In this case, the with block only indicates where warnings should be caught.

@gidden
Copy link
Contributor

gidden commented May 24, 2017 via email

@shoyer shoyer merged commit 5007961 into pydata:master May 24, 2017
@shoyer shoyer deleted the fix-pytest-warnings branch May 24, 2017 16:41
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.

2 participants