Skip to content

(temporary) add pyyaml to unittest requirements #5099

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Dec 14, 2021
Merged

(temporary) add pyyaml to unittest requirements #5099

merged 3 commits into from
Dec 14, 2021

Conversation

pmeier
Copy link
Collaborator

@pmeier pmeier commented Dec 14, 2021 •
edited
Loading

pytorch/pytorch#68772 added pyyaml as dependency and with pytorch/pytorch#68773 it is now a hard dependency when using torch.jit.load. Since it is not listed in torch's dependency, we need to install it manually.

I propose to merge this PR now and revert it as soon as this is resolved upstream.

cc @seemethere @tugsbayasgalan @gmagogsfm

@facebook-github-bot
Copy link

facebook-github-bot commented Dec 14, 2021 •
edited
Loading

💊 CI failures summary and remediations

As of commit 4d80324 (more details on the Dr. CI page):

💚 💚 Looks good so far! There are no failures yet. 💚 💚

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@jdsgomes jdsgomes requested review from jdsgomes and removed request for jdsgomes December 14, 2021 15:34
fmassa
fmassa approved these changes Dec 14, 2021
View reviewed changes
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.

Ok to unblock, but let's not forget to remove it once PyTorch fixes it.

Can you add a comment on each line so that we remember to remove this?

@fmassa
Copy link
Member

fmassa commented Dec 14, 2021

Issue to track this pytorch/pytorch#69905

@malfet malfet merged commit 743fe1a into main Dec 14, 2021
@malfet malfet deleted the fix-ci branch December 14, 2021 17:04
@github-actions
Copy link

Hey @malfet!

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

@malfet malfet mentioned this pull request Dec 14, 2021
Closed
@malfet malfet added bug and removed bug labels Dec 14, 2021
@malfet
Copy link
Contributor

malfet commented Dec 14, 2021 •
edited
Loading

Hmm, @pmeier looks like you've added dependency issue label which is mentioned in process_commit.py, why does it still complain about missing labels?

@pmeier
Copy link
Collaborator Author

pmeier commented Dec 14, 2021

It complains, because it only found a label from the secondary category. Maybe we should change the message to reflect that we should have at least one label from each category? cc @NicolasHug

@cccclai
Copy link

cccclai commented Dec 18, 2021

pytorch/pytorch#69905 is resolved. pyyaml can be safely removed.

datumbox added a commit that referenced this pull request Dec 18, 2021
@datumbox
Revert "(temporary) add pyyaml to unittest requirements (#5099)"
3bdb002 
This reverts commit 743fe1a.
@datumbox datumbox mentioned this pull request Dec 18, 2021
Merged
datumbox added a commit that referenced this pull request Dec 19, 2021
@datumbox
Revert "(temporary) add pyyaml to unittest requirements (#5099)" (#…
40be657 
…5112)

This reverts commit 743fe1a.
@datumbox datumbox added the revert(ed) For reverted PRs, and PRs that revert other PRs label Dec 19, 2021
facebook-github-bot pushed a commit that referenced this pull request Dec 21, 2021
@datumbox @facebook-github-bot
[fbsync] (temporary) add pyyaml to unittest requirements (#5099)
d626ac8 
Summary:
* (temporary) add pyyaml to unittest requirements

* add actionable comments

Reviewed By: prabhat00155

Differential Revision: D33253465

fbshipit-source-id: 7e254dd54431c8cb2f0afc9ef26f6b04183c9df6
facebook-github-bot pushed a commit that referenced this pull request Dec 21, 2021
@datumbox @facebook-github-bot
[fbsync] Revert "(temporary) add pyyaml to unittest requirements (#…
d2b6168 
…5099)" (#5112)

Summary: This reverts commit 743fe1a.

Reviewed By: prabhat00155

Differential Revision: D33253467

fbshipit-source-id: 70dd77709f274478f4a36ff3f04eccf16cbbf21e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@malfet malfet malfet approved these changes

@fmassa fmassa fmassa approved these changes

Assignees
No one assigned
Labels
ciflow/default cla signed dependency issue module: ci revert(ed) For reverted PRs, and PRs that revert other PRs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@pmeier @facebook-github-bot @fmassa @malfet @cccclai @datumbox