Skip to content

Re-enable TestNamedTensor.test_big_tensor_repr #29407

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

Closed
wants to merge 2 commits into from

Conversation

zou3519
Copy link
Contributor

@zou3519 zou3519 commented Nov 7, 2019

Stack from ghstack:

Fixes #27753.

The bug was that random tensors print subtly differently. This causes
the "names=" tag to appear in slightly different places; sometimes it is
on the same line as the data, sometimes it is on different lines.

For this test, we wanted to know the following:

  • printing a big named tensor's repr doesn't crash
  • a big named tensor's repr shows the names

This PR changes the test to check those two things.

Test Plan:

  • run existing tests

Differential Revision: D18428657

Fixes #27753.

The bug was that random tensors print subtly differently. This causes
the "names=" tag to appear in slightly different places; sometimes it is
on the same line as the data, sometimes it is on different lines.

For this test, we wanted to know the following:
- printing a big named tensor's repr doesn't crash
- a big named tensor's repr shows the names

This PR changes the test to check those two things.

Test Plan:
- run existing tests
zou3519 added a commit that referenced this pull request Nov 7, 2019
Fixes #27753.

The bug was that random tensors print subtly differently. This causes
the "names=" tag to appear in slightly different places; sometimes it is
on the same line as the data, sometimes it is on different lines.

For this test, we wanted to know the following:
- printing a big named tensor's repr doesn't crash
- a big named tensor's repr shows the names

This PR changes the test to check those two things.

Test Plan:
- run existing tests

ghstack-source-id: ae5d443
Pull Request resolved: #29407
@zou3519 zou3519 requested review from izdeby, nairbv and gchanan November 7, 2019 21:21
expected = "{}, names={})".format(repr(unnamed_tensor)[:-1], named_tensor.names)
self.assertEqual(repr(named_tensor), expected)
names_tag = 'names={}'.format(named_tensor.names)
self.assertTrue(names_tag in repr(named_tensor))
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: we could use assertRegexpMatches so that any error message would print both of the non-matching strings

Copy link
Contributor Author

@zou3519 zou3519 Nov 11, 2019

Choose a reason for hiding this comment

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

EDIT: That's a good idea, let me try that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It turns out there is something called assertIn that solves this problem

Fixes #27753.

The bug was that random tensors print subtly differently. This causes
the "names=" tag to appear in slightly different places; sometimes it is
on the same line as the data, sometimes it is on different lines.

For this test, we wanted to know the following:
- printing a big named tensor's repr doesn't crash
- a big named tensor's repr shows the names

This PR changes the test to check those two things.

Test Plan:
- run existing tests
zou3519 added a commit that referenced this pull request Nov 11, 2019
Fixes #27753.

The bug was that random tensors print subtly differently. This causes
the "names=" tag to appear in slightly different places; sometimes it is
on the same line as the data, sometimes it is on different lines.

For this test, we wanted to know the following:
- printing a big named tensor's repr doesn't crash
- a big named tensor's repr shows the names

This PR changes the test to check those two things.

Test Plan:
- run existing tests

ghstack-source-id: 88ca7b3
Pull Request resolved: #29407
@facebook-github-bot
Copy link
Contributor

@zou3519 merged this pull request in cedca37.

@facebook-github-bot facebook-github-bot deleted the gh/zou3519/213/head branch November 15, 2019 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants