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

Remove reference_video_tensor() fixture #62

Closed
wants to merge 2 commits into from

Conversation

NicolasHug
Copy link
Member

@NicolasHug NicolasHug commented Jul 8, 2024

This PR removes reference_video_tensor() because its unnecessary.

I assume the use of this fixture was used to cache the corresponding returned value. This is unnecessary for 2 reasons:

  • The fixture wasn't even cached anyway, because its scope wasn't set to "session". You can verify that by printing something within the fixture, and running tests with pytest -s. You'll see the print statement being executed each time, proving that no caching was involved.
  • Getting the return value is dead cheap and doesn't need to be cached in the first place. Proof:
[ins] In [1]: import test_utils
tes
[nav] In [2]: %timeit test_utils.NASA_VIDEO.to_tensor()
55.8 µs ± 3.76 µs per loop (mean ± std. dev. of 7 runs, 10,000 loops each)

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jul 8, 2024
@facebook-github-bot
Copy link
Contributor

@NicolasHug has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

1 similar comment
@facebook-github-bot
Copy link
Contributor

@NicolasHug has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@NicolasHug has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@NicolasHug merged this pull request in 508eee1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants