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

Use packaging.version instead of distutils Version classes #3700

Merged
merged 4 commits into from Sep 14, 2022

Conversation

njzjz
Copy link
Contributor

@njzjz njzjz commented Sep 14, 2022

Checklist before submitting

  • Did you read the contributor guide?
  • Did you update the docs?
  • Did you write any tests to validate this change?
  • Did you update the CHANGELOG, if this change affects users?

Description

Resolves the following warning:

/usr/local/lib/python3.10/dist-packages/horovod/tensorflow/elastic.py:28: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead.

Review process to land

  1. All tests and other checks must succeed.
  2. At least one member of the technical steering committee must review and approve.
  3. If any member of the technical steering committee requests changes, they must be addressed.

@maxhgerlach
Copy link
Collaborator

Thank you for the contribution @njzjz! It's good to keep the code base up to date and to clear warnings.

If you feel like continuing with this and if you have the time, there are about 40 further usages of distutils.versions in Horovod code, tests, and examples..

Resolves the following warning:
  /usr/local/lib/python3.10/dist-packages/horovod/tensorflow/elastic.py:28: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead.

Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
@njzjz
Copy link
Contributor Author

njzjz commented Sep 14, 2022

Hi @maxhgerlach, I have replaced all of distutils.version.

Note: Per documentation distutils will be removed in Python 3.12.

@maxhgerlach maxhgerlach linked an issue Sep 14, 2022 that may be closed by this pull request
Copy link
Collaborator

@maxhgerlach maxhgerlach left a comment

Choose a reason for hiding this comment

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

Great work @njzjz! Let's just wait for a full run of the CI pipeline.

@github-actions
Copy link

Unit Test Results

  1 087 files  ±0    1 087 suites  ±0   11h 17m 18s ⏱️ + 4m 26s
     813 tests ±0       755 ✔️ ±0       58 💤 ±0  0 ±0 
21 410 runs  ±0  15 042 ✔️ ±0  6 368 💤 ±0  0 ±0 

Results for commit 09d554b. ± Comparison against base commit 94529cc.

@github-actions
Copy link

Unit Test Results (with flaky tests)

  1 198 files   -   44    1 198 suites   - 44   12h 1m 12s ⏱️ + 14m 29s
     813 tests ±    0       755 ✔️ +    1       58 💤 ±    0  0  - 1 
23 722 runs   - 645  16 378 ✔️  - 421  7 344 💤  - 223  0  - 1 

Results for commit 09d554b. ± Comparison against base commit 94529cc.

@maxhgerlach
Copy link
Collaborator

Tests are fines, only unrelated failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Migrate distutils to packaging.version
2 participants