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.

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

Fix the pytest version to be less than 8.0.1 #2461

Merged
merged 2 commits into from
Feb 23, 2024
Merged

Fix the pytest version to be less than 8.0.1 #2461

merged 2 commits into from
Feb 23, 2024

Conversation

BenjaminBossan
Copy link
Member

What does this PR do?

Fixes the pytest version.

We're getting errors such as this one:

https://github.com/huggingface/accelerate/actions/runs/7958684877/job/21725397566?pr=2450

@BenjaminBossan
Fix the pytest version to be less than 8.0.0
2bd8170 
We're getting errors such as:

> /opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/transformers/testing_utils.py:129: in <module>
>     from _pytest.doctest import (
> E   ImportError: cannot import name 'import_path' from '_pytest.doctest' (/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/_pytest/doctest.py)
@HuggingFaceDocBuilderDev
Copy link

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@BenjaminBossan
Copy link
Member Author

Not sure whom to ping, as Zach is off this week, maybe @ydshieh with reference to huggingface/transformers#28758.

@BenjaminBossan BenjaminBossan mentioned this pull request Feb 21, 2024
Merged
fxmarty
fxmarty approved these changes Feb 21, 2024
View reviewed changes
Copy link
Contributor

@fxmarty fxmarty left a comment

Choose a reason for hiding this comment

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

LGTM

fxmarty
fxmarty reviewed Feb 21, 2024
View reviewed changes
setup.py Outdated Show resolved Hide resolved
@SunMarc SunMarc self-requested a review February 21, 2024 14:22
SunMarc
SunMarc approved these changes Feb 21, 2024
View reviewed changes
Copy link
Member

@SunMarc SunMarc left a comment

Choose a reason for hiding this comment

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

LGTM with @fxmarty comments. We shouldn't have issues with 8.0.0. 8.0.1 is the problematic one. In transformers, they pinned the pytest version for the tests to <8.0.1

@BenjaminBossan
Copy link
Member Author

In the linked PR, the version was pinned to <8.0.0, not sure why it's done partly including and partly excluding 8.0.0. I can change this to <=8.0.0 if we're fine with that.

@fxmarty
Copy link
Contributor

fxmarty commented Feb 21, 2024

@BenjaminBossan I don't think it really matters, let's leave it like is

@SunMarc @fxmarty
Update setup.py
f2cb8a1 
Co-authored-by: fxmarty <9808326+fxmarty@users.noreply.github.com>
@SunMarc SunMarc changed the title Fix the pytest version to be less than 8.0.0 Fix the pytest version to be less than 8.0.1 Feb 23, 2024
@SunMarc
Copy link
Member

SunMarc commented Feb 23, 2024 •
edited
Loading

In the huggingface/transformers#28758, the version was pinned to <8.0.0, not sure why it's done partly including and partly excluding 8.0.0

In transformers, they had other issues with 8.0.0 for some tests. I will merge this since some checks are failing because without this specific PR. Thanks again @BenjaminBossan for your prompt fix !

@SunMarc SunMarc merged commit f20445d into huggingface:main Feb 23, 2024
23 checks passed
@BenjaminBossan BenjaminBossan deleted the MNT-fix-pytest-value-lt-8.0.0 branch February 26, 2024 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fxmarty fxmarty fxmarty approved these changes

@SunMarc SunMarc SunMarc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@BenjaminBossan @HuggingFaceDocBuilderDev @fxmarty @SunMarc