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 unnecessary dependency from macOS/Conda binaries #8077

Merged
merged 3 commits into from Oct 30, 2023

Conversation

mthrok
Copy link
Contributor

@mthrok mthrok commented Oct 27, 2023

This commit updates the dependency description on Conda package,
so that FFmpeg is installed only on platforms on which torchvision comes with the corresponding extension.

  • Currently, FFmpeg integration is enabled only for Linux.

vision/setup.py

Lines 366 to 367 in a8ebd0b

if sys.platform != "linux" or (sys.version_info.major == 3 and sys.version_info.minor == 9):
has_ffmpeg = False

  • For binary packaging, even for Linux, it is excluded in wheel. (due to the lack of build-time dependency in CI)

https://github.com/pytorch/vision/actions/runs/6678010959/job/18148611653#step:14:106

  • The Linux/Conda package is still built with FFmpeg after this commit

https://github.com/pytorch/vision/actions/runs/6695315792/job/18190638421?pr=8077#step:9:199

Update the dependency.

Currently only Linux + conda ships FFmpeg integration.
@pytorch-bot
Copy link

pytorch-bot bot commented Oct 27, 2023

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/8077

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (16 Unrelated Failures)

As of commit 0f7e665 with merge base a8ebd0b (image):

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@mthrok mthrok marked this pull request as draft October 27, 2023 17:50
@mthrok mthrok changed the title WIP: Update meta.yaml WIP/TEST: Update meta.yaml Oct 27, 2023
@mthrok mthrok changed the title WIP/TEST: Update meta.yaml Remove unnecessary dependency in macOS/Conda Oct 30, 2023
@mthrok mthrok marked this pull request as ready for review October 30, 2023 16:28
@mthrok mthrok changed the title Remove unnecessary dependency in macOS/Conda Remove unnecessary dependency from macOS/Conda binaries Oct 30, 2023
@mthrok
Copy link
Contributor Author

mthrok commented Oct 30, 2023

@NicolasHug Review please.

Copy link
Collaborator

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @mthrok!

@mthrok
Copy link
Contributor Author

mthrok commented Oct 30, 2023

@pmeier Thanks. I don't have a merge privilege. Could you merge it?

@pmeier pmeier merged commit f69eee6 into pytorch:main Oct 30, 2023
34 of 49 checks passed
@github-actions
Copy link

Hey @pmeier!

You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py

@mthrok mthrok deleted the patch-1 branch October 31, 2023 00:28
facebook-github-bot pushed a commit that referenced this pull request Nov 16, 2023
Summary: Co-authored-by: Philip Meier <github.pmeier@posteo.de>

Reviewed By: vmoens

Differential Revision: D51391970

fbshipit-source-id: aa36c68e85a5fc3a348f44f35f7d7f618ee24d57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants