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

[examples] adding the references for the new video API #2770

Merged
merged 9 commits into from
Oct 8, 2020

Conversation

bjuncek
Copy link
Contributor

@bjuncek bjuncek commented Oct 7, 2020

Simple IPYNB reference for using the new video API.

@codecov
Copy link

codecov bot commented Oct 7, 2020

Codecov Report

Merging #2770 into master will increase coverage by 0.55%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2770      +/-   ##
==========================================
+ Coverage   73.15%   73.70%   +0.55%     
==========================================
  Files          96       96              
  Lines        8396     8695     +299     
  Branches     1315     1408      +93     
==========================================
+ Hits         6142     6409     +267     
- Misses       1855     1877      +22     
- Partials      399      409      +10     
Impacted Files Coverage Δ
torchvision/io/image.py 86.66% <0.00%> (+1.88%) ⬆️
torchvision/transforms/transforms.py 83.76% <0.00%> (+2.51%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6eff0a4...50e1d06. Read the comment docs.

@bjuncek bjuncek changed the title [references] adding the references for the new video API [examples] adding the references for the new video API Oct 7, 2020
Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

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

Thanks for the changes!

I think there are a few minor bugs, but once those are fixed we can merge this to unblock and move forward

" # get random sample\n",
" path, target = random.choice(self.samples)\n",
" # get video object\n",
" vid = torch.classes.torchvision.Video(path, \"video\")\n",
Copy link
Member

Choose a reason for hiding this comment

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

nit: update to the final path of Video

@@ -0,0 +1,672 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

Can we change the name of the video to follow snake_case? None of the files in the repo (apart from from C++ files) use CamelCase

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you mean the name of the file?
done if so

Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

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

Approving to move forward with this

@fmassa fmassa merged commit 2cc20d7 into pytorch:master Oct 8, 2020
bryant1410 pushed a commit to bryant1410/vision-1 that referenced this pull request Nov 22, 2020
* gitignore now supports IPYNB aux files

* Adding the reference ipython notebook

* Update location

* link fix

* Add autodownload for colab

* rename and address fmassa's comments

* nitpicks

* nitpicks

* Apply suggestions from code review

Co-authored-by: Francisco Massa <[email protected]>
vfdev-5 pushed a commit to Quansight/vision that referenced this pull request Dec 4, 2020
* gitignore now supports IPYNB aux files

* Adding the reference ipython notebook

* Update location

* link fix

* Add autodownload for colab

* rename and address fmassa's comments

* nitpicks

* nitpicks

* Apply suggestions from code review

Co-authored-by: Francisco Massa <[email protected]>
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