Skip to content

Removes pinned version for pytest #2158

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

Merged
merged 2 commits into from
May 2, 2025
Merged

Conversation

syed-ahmed
Copy link
Contributor

Unless 7.4.0 is actually required, we should remove this so that pip is able to resolve dependencies in other environments:

ERROR: Cannot install pytest==7.4.0 because these package versions have conflicting dependencies.                                                                                                    
                                                                                                                                                                                                     
The conflict is caused by:                                                                                                                                                                           
    The user requested pytest==7.4.0                                                                                                                                                                 
    The user requested (constraint) pytest==8.1.1                                                                                                                                                    

To fix this you could try to:
1. loosen the range of package versions you've specified
2. remove package versions to allow pip to attempt to solve the dependency conflict

ERROR: ResolutionImpossible: for help visit https://pip.pypa.io/en/latest/topics/dependency-resolution/#dealing-with-dependency-conflicts

Copy link

pytorch-bot bot commented May 1, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/ao/2158

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

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

❌ 1 New Failure

As of commit e0ba155 with merge base 12467d2 (image):

NEW FAILURE - The following job has failed:

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

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 1, 2025
@jerryzh168
Copy link
Contributor

seems to be OK to me, cc @msaroufim @drisspg @atalman ?

@jerryzh168 jerryzh168 added the topic: not user facing Use this tag if you don't want this PR to show up in release notes label May 1, 2025
@msaroufim
Copy link
Member

IIRC we wanted to get rid of pytest entirely, might be time to accelerate the transition?

@jerryzh168
Copy link
Contributor

@msaroufim I see, then we'll need to finish this one: #1621

@drisspg
Copy link
Contributor

drisspg commented May 2, 2025

We use pretty minimal set of the PyTest features so I think it is this is fine for now, unless the failures in this PR are in fact caused by unpinning

@jerryzh168
Copy link
Contributor

CI errors might be related I think, @syed-ahmed can you confirm

@syed-ahmed
Copy link
Contributor Author

@jerryzh168 Yeah, looks like the errors are real. It's happening because pytest does have a new requirement: pytorch/pytorch#127517. Looks like we workaround it: pytorch/pytorch#136158 and that's why the CI passes for PyTorch 2.6 and 2.7.

I've added a dev-requirements-overrides: "s/^pytest$/pytest==7.4.0/" in the github workflow of the 2.5 tests, and we can remove it after the next PyTorch release. Is that ok?

@msaroufim
Copy link
Member

msaroufim commented May 2, 2025

@metascroy @manuelcandales seems like the MPS jobs are failing consistently on this and other PRs

@msaroufim msaroufim merged commit 5722d9a into pytorch:main May 2, 2025
17 of 18 checks passed
@msaroufim
Copy link
Member

Actually I messed up merging this, I didn't realize sentencepiece has some test utilities at build time

@msaroufim
Copy link
Member

Actually I did not mess up seems like it's a flake and occurs even if i downgrade pytest again #2166

Main is healthy

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 Facebook bot. Authors need to sign the CLA before a PR can be reviewed. topic: not user facing Use this tag if you don't want this PR to show up in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants