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

Update pytest import_path location #29154

Merged
merged 3 commits into from
Mar 5, 2024
Merged

Update pytest import_path location #29154

merged 3 commits into from
Mar 5, 2024

Conversation

loadams
Copy link
Contributor

@loadams loadams commented Feb 20, 2024 •
edited
Loading

What does this PR do?

Fixes location of import_path from pytest from _pytest.doctest to _pytest.pathlib when using PyTest 8.0.1+ since it is finally deprecated from being in _pytest.doctest. It is provided in _pytest.pathlib from at least 7.2.0+ so we do not need to modify the supported pytest range in setup.py

Tested here in DeepSpeed and tests appear to be passing.

Fixes: #29155

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@muellerzr and @pacman100

@loadams loadams mentioned this pull request Feb 20, 2024
Merged
pacman100
pacman100 approved these changes Feb 21, 2024
View reviewed changes
Copy link
Contributor

@pacman100 pacman100 left a comment

Choose a reason for hiding this comment

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

Thank you @loadams for the fixes! Pinging @ydshieh and @amyeroberts for a second look.

@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.

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

@amyeroberts amyeroberts 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 for fixing!

Let's wait for a final review from @ydshieh - as he'll be the one who has to handle anything in case this breaks something!

@loadams
Copy link
Contributor Author

loadams commented Feb 22, 2024

Thanks @amyeroberts! @ydshieh - could you review when you have time, I'd like to update DeepSpeed to the latest here when possible.

@loadams
Copy link
Contributor Author

loadams commented Feb 26, 2024

Hi @ydshieh, curious if you'd have some time to review this PR?

@loadams
Copy link
Contributor Author

loadams commented Mar 4, 2024

Hi @amyeroberts @pacman100 - gentle ping on being able to merge this PR?

@samruds samruds mentioned this pull request Mar 4, 2024
Closed
@amyeroberts
Copy link
Collaborator

I'm going to merge, as I'm pretty sure it's OK - @ydshieh for reference

@amyeroberts amyeroberts merged commit 9c5e560 into huggingface:main Mar 5, 2024
20 checks passed
@loadams
Copy link
Contributor Author

loadams commented Mar 5, 2024

Thanks @amyeroberts!

AdrianAbeyta pushed a commit to ROCm/transformers that referenced this pull request Mar 5, 2024
@loadams
Update pytest import_path location (huggingface#29154)
c9465f0 
* Update to pull function from proper lib

* Fix ruff formatting error

* Remove accidently added file
@AdrianAbeyta AdrianAbeyta mentioned this pull request Mar 5, 2024
Merged
damithsenanayake pushed a commit to damithsenanayake/transformers that referenced this pull request Mar 7, 2024
@loadams @damithsenanayake
Update pytest import_path location (huggingface#29154)
5871fab 
* Update to pull function from proper lib

* Fix ruff formatting error

* Remove accidently added file
github-merge-queue bot pushed a commit to microsoft/DeepSpeed that referenced this pull request Apr 5, 2024
@loadams
Update pytest and transformers with fixes for pytest>= 8.0.0 (#5164)
3f4f7b6 
The underlying issue preventing us from using pytest 8.0.1+ is in
transformers. Here is a [PR that fixes the
issue](huggingface/transformers#29154) that will
need to be merged and then a new release of transformers before we can
complete this PR.

No impact to nv-nightly, run
[here](https://github.com/microsoft/DeepSpeed/actions/runs/8575192380/job/23503659644)
Currently errors on nv-inference from [the
PR](https://github.com/microsoft/DeepSpeed/actions/runs/8575187611/job/23503647378),
but the same tests are broken in
[master](https://github.com/microsoft/DeepSpeed/actions/runs/8575185551/job/23503640238).
github-merge-queue bot pushed a commit to microsoft/DeepSpeed that referenced this pull request Apr 8, 2024
@loadams
Update pytest and transformers with fixes for pytest>= 8.0.0 (#5164)
72bd275 
The underlying issue preventing us from using pytest 8.0.1+ is in
transformers. Here is a [PR that fixes the
issue](huggingface/transformers#29154) that will
need to be merged and then a new release of transformers before we can
complete this PR.

No impact to nv-nightly, run
[here](https://github.com/microsoft/DeepSpeed/actions/runs/8575192380/job/23503659644)
Currently errors on nv-inference from [the
PR](https://github.com/microsoft/DeepSpeed/actions/runs/8575187611/job/23503647378),
but the same tests are broken in
[master](https://github.com/microsoft/DeepSpeed/actions/runs/8575185551/job/23503640238).
@astachowiczhabana astachowiczhabana mentioned this pull request Apr 23, 2024
Merged
rraminen pushed a commit to ROCm/DeepSpeed that referenced this pull request May 9, 2024
@loadams @rraminen
Update pytest and transformers with fixes for pytest>= 8.0.0 (microso…
c378288 
…ft#5164)

The underlying issue preventing us from using pytest 8.0.1+ is in
transformers. Here is a [PR that fixes the
issue](huggingface/transformers#29154) that will
need to be merged and then a new release of transformers before we can
complete this PR.

No impact to nv-nightly, run
[here](https://github.com/microsoft/DeepSpeed/actions/runs/8575192380/job/23503659644)
Currently errors on nv-inference from [the
PR](https://github.com/microsoft/DeepSpeed/actions/runs/8575187611/job/23503647378),
but the same tests are broken in
[master](https://github.com/microsoft/DeepSpeed/actions/runs/8575185551/job/23503640238).
itazap pushed a commit that referenced this pull request May 14, 2024
@loadams
Update pytest import_path location (#29154)
8cb491e 
* Update to pull function from proper lib

* Fix ruff formatting error

* Remove accidently added file
umchand pushed a commit to umchand/DeepSpeed that referenced this pull request May 20, 2024
@loadams @umchand
Update pytest and transformers with fixes for pytest>= 8.0.0 (microso…
face3b3 
…ft#5164)

The underlying issue preventing us from using pytest 8.0.1+ is in
transformers. Here is a [PR that fixes the
issue](huggingface/transformers#29154) that will
need to be merged and then a new release of transformers before we can
complete this PR.

No impact to nv-nightly, run
[here](https://github.com/microsoft/DeepSpeed/actions/runs/8575192380/job/23503659644)
Currently errors on nv-inference from [the
PR](https://github.com/microsoft/DeepSpeed/actions/runs/8575187611/job/23503647378),
but the same tests are broken in
[master](https://github.com/microsoft/DeepSpeed/actions/runs/8575185551/job/23503640238).
dbyoung18 pushed a commit to dbyoung18/DeepSpeed that referenced this pull request Jun 11, 2024
@loadams @dbyoung18
Update pytest and transformers with fixes for pytest>= 8.0.0 (microso…
74e64d8 
…ft#5164)

The underlying issue preventing us from using pytest 8.0.1+ is in
transformers. Here is a [PR that fixes the
issue](huggingface/transformers#29154) that will
need to be merged and then a new release of transformers before we can
complete this PR.

No impact to nv-nightly, run
[here](https://github.com/microsoft/DeepSpeed/actions/runs/8575192380/job/23503659644)
Currently errors on nv-inference from [the
PR](https://github.com/microsoft/DeepSpeed/actions/runs/8575187611/job/23503647378),
but the same tests are broken in
[master](https://github.com/microsoft/DeepSpeed/actions/runs/8575185551/job/23503640238).
@kiersten-stokes kiersten-stokes mentioned this pull request Jun 27, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amyeroberts amyeroberts amyeroberts approved these changes

@pacman100 pacman100 pacman100 approved these changes

@fxmarty fxmarty fxmarty approved these changes

@ydshieh ydshieh Awaiting requested review from ydshieh

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

Successfully merging this pull request may close these issues.

PyTest import error
5 participants
@loadams @HuggingFaceDocBuilderDev @amyeroberts @fxmarty @pacman100