[documentation] video API documentation and wrapper #2778
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…n into bkorbar/videoAPI/pythonreg
Codecov Report
@@ Coverage Diff @@
## master #2778 +/- ##
==========================================
+ Coverage 73.18% 73.45% +0.27%
==========================================
Files 96 96
Lines 8417 8541 +124
Branches 1316 1338 +22
==========================================
+ Hits 6160 6274 +114
- Misses 1857 1873 +16
+ Partials 400 394 -6
Continue to review full report at Codecov.
|
fmassa
reviewed
Oct 9, 2020
Comment on lines
+30
to
+32
| if _HAS_VIDEO_OPT: | ||
|
|
||
| class Video: |
There was a problem hiding this comment.
For a follow-up PR: given that we are wrapping our C++ class in a python class, we don't need to have this as is, and we can instead implement everything outside the guard, but have a guard at instantiation time.
Something like
if _HAS_VIDEO_OPT:
def _has_video_opt():
return True
else:
def _has_video_opt():
return False
class Video:
def __init__(self, path, stream):
# check if video_opt is available
if not _has_video_opt():
raise RuntimeError(...)
...In here, we create a function _has_video_opt to make the class work with torchscript (which doesn't support globals).
There was a problem hiding this comment.
added to the list of issues to address
Co-authored-by: Francisco Massa <fvsmassa@gmail.com>
bryant1410
pushed a commit
to bryant1410/vision-1
that referenced
this pull request
Nov 22, 2020
* initial API documentation attempt * test the docs * initial commit * updating test to match the registration * adding the warning on unsucessful import * Try to do conditional import * Simple fix? * clearing up docs * docstring commit * Adding types in arguments Co-authored-by: Francisco Massa <fvsmassa@gmail.com> * reverting warning commit * addressing Francisco's comments * Apply suggestions from code review Co-authored-by: Francisco Massa <fvsmassa@gmail.com> * Revert "reverting warning commit" This reverts commit bd1a3dd. * Revert "adding the warning on unsucessful import" This reverts commit afef7df. * remove warnings import Co-authored-by: Francisco Massa <fvsmassa@gmail.com>
vfdev-5
pushed a commit
to Quansight/vision
that referenced
this pull request
Dec 4, 2020
* initial API documentation attempt * test the docs * initial commit * updating test to match the registration * adding the warning on unsucessful import * Try to do conditional import * Simple fix? * clearing up docs * docstring commit * Adding types in arguments Co-authored-by: Francisco Massa <fvsmassa@gmail.com> * reverting warning commit * addressing Francisco's comments * Apply suggestions from code review Co-authored-by: Francisco Massa <fvsmassa@gmail.com> * Revert "reverting warning commit" This reverts commit bd1a3dd. * Revert "adding the warning on unsucessful import" This reverts commit afef7df. * remove warnings import Co-authored-by: Francisco Massa <fvsmassa@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To be seen if it has impact on performance due to additional python binding