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 pyproject.toml #22507

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Update pyproject.toml #22507

wants to merge 13 commits into from

Conversation

jchen351
Copy link
Contributor

Description

Update pyproject.toml and remove outdate python version support. Adding python313.

tianleiwu
tianleiwu previously approved these changes Oct 21, 2024
View reviewed changes
Copy link
Contributor

@tianleiwu tianleiwu left a comment

Choose a reason for hiding this comment

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

please merge main to fix build.

@jchen351 jchen351 requested a review from a team as a code owner October 21, 2024 20:45
@jchen351
Copy link
Contributor Author

Black dosen't support py313.

@jchen351 jchen351 closed this Oct 21, 2024
snnn
snnn reviewed Oct 21, 2024
View reviewed changes
.github/workflows/lint.yml Outdated Show resolved Hide resolved
snnn
snnn reviewed Oct 21, 2024
View reviewed changes
pyproject.toml Outdated Show resolved Hide resolved
@jchen351 jchen351 reopened this Oct 21, 2024
tianleiwu
tianleiwu previously approved these changes Oct 22, 2024
View reviewed changes
@snnn
Copy link
Member

snnn commented Oct 22, 2024

Why the linter runner found so many issues?

@snnn snnn requested a review from justinchuby October 22, 2024 04:20
@snnn
Copy link
Member

snnn commented Oct 22, 2024

@justinchuby , please help review.

@tianleiwu
Copy link
Contributor

Why the linter runner found so many issues?
I think it is because the target changes from py38 to py310. May be try run lintrunner -a to format files and add those changed files.

@justinchuby

This comment was marked as resolved.

Sign in to view
justinchuby
justinchuby reviewed Oct 22, 2024
View reviewed changes
pyproject.toml Outdated Show resolved Hide resolved
@jchen351 @justinchuby
Update pyproject.toml
f1003ea 
Co-authored-by: Justin Chu <[email protected]>
@justinchuby
Copy link
Contributor

justinchuby commented Oct 22, 2024 •
edited
Loading

Sorry let me update my previous comment:

  1. It looks like all that's updated is for the formatter and linters. In this case, I would: do the black and ruff changes and update the desired versions. Apply lintrunner changes (lintrunner f --all-files) and commit them here in the same PR (this PR). These are linter changes.
  2. For package metadata changes, it looks like you need to update

    onnxruntime/setup.py

    Lines 539 to 545 in 62f99d8

    "Programming Language :: Python :: 3 :: Only",
    "Programming Language :: Python :: 3.7",
    "Programming Language :: Python :: 3.8",
    "Programming Language :: Python :: 3.9",
    "Programming Language :: Python :: 3.10",
    "Programming Language :: Python :: 3.11",
    "Programming Language :: Python :: 3.12",

@jchen351
Copy link
Contributor Author

Sorry let me update my previous comment:

  1. It looks like all that's updated is for the formatter and linters. In this case, I would: do the black and ruff changes and update the desired versions. Apply lintrunner changes (lintrunner f --all-files) and commit them here in the same PR (this PR). These are linter changes.
  2. For package metadata changes, it looks like you need to update

    onnxruntime/setup.py

    Lines 539 to 545 in 62f99d8

    "Programming Language :: Python :: 3 :: Only",
    "Programming Language :: Python :: 3.7",
    "Programming Language :: Python :: 3.8",
    "Programming Language :: Python :: 3.9",
    "Programming Language :: Python :: 3.10",
    "Programming Language :: Python :: 3.11",
    "Programming Language :: Python :: 3.12",

    Applying lintrunner f --all-files will modify 300+ files.

jchen351
jchen351 commented Oct 22, 2024
View reviewed changes
setup.py Show resolved Hide resolved
github-actions[bot]
github-actions bot requested changes Oct 22, 2024
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You can commit the suggested changes from lintrunner.

include/onnxruntime/core/common/exceptions.h Show resolved Hide resolved
include/onnxruntime/core/common/exceptions.h Show resolved Hide resolved
include/onnxruntime/core/framework/stream_handles.h Show resolved Hide resolved
include/onnxruntime/core/platform/Barrier.h Show resolved Hide resolved
include/onnxruntime/core/platform/EigenNonBlockingThreadPool.h Show resolved Hide resolved
onnxruntime/core/providers/cpu/tensor/expand.cc Show resolved Hide resolved
onnxruntime/core/providers/cuda/cuda_execution_provider.cc Show resolved Hide resolved
onnxruntime/core/providers/cuda/cuda_graph.h Show resolved Hide resolved
onnxruntime/core/providers/cuda/cuda_graph.h Show resolved Hide resolved
onnxruntime/core/providers/cuda/cuda_profiler.h Show resolved Hide resolved
github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 22, 2024
View reviewed changes
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

lintrunner found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 22, 2024
View reviewed changes
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@justinchuby
Copy link
Contributor

Modifying 300+ files should be fine, because they are all auto fixes

@jchen351
Merge branch 'main' into Cjian/pytoml
210741a 
# Conflicts:
#	onnxruntime/core/providers/cpu/cpu_execution_provider.cc
@jchen351
apply lintrunner f --all-files
4c49f45
github-actions[bot]
github-actions bot previously requested changes Oct 23, 2024
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You can commit the suggested changes from lintrunner.

include/onnxruntime/core/platform/EigenNonBlockingThreadPool.h Show resolved Hide resolved
include/onnxruntime/core/platform/EigenNonBlockingThreadPool.h Show resolved Hide resolved
onnxruntime/core/optimizer/transpose_optimization/optimizer_api.h Show resolved Hide resolved
onnxruntime/core/optimizer/transpose_optimization/optimizer_api.h Show resolved Hide resolved
onnxruntime/core/platform/path_lib.h Show resolved Hide resolved
onnxruntime/core/providers/cpu/tensor/expand.cc Show resolved Hide resolved
onnxruntime/core/providers/cuda/cuda_execution_provider.cc Show resolved Hide resolved
onnxruntime/core/providers/cuda/cuda_graph.h Show resolved Hide resolved
onnxruntime/core/providers/cuda/cuda_graph.h Show resolved Hide resolved
onnxruntime/core/providers/cuda/cuda_profiler.h Show resolved Hide resolved
@jchen351 @github-actions
Update onnxruntime/core/providers/cpu/cpu_execution_provider.cc
d6e3e02 
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@jchen351 jchen351 dismissed github-actions[bot]’s stale review October 24, 2024 20:00

Conflict with lintrunner

@jchen351
Merge branch 'main' into Cjian/pytoml
a0225d9 
# Conflicts:
#	orttraining/orttraining/models/pipeline_poc/main.cc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@snnn snnn snnn left review comments

@tianleiwu tianleiwu tianleiwu left review comments

@github-actions github-actions[bot] github-actions[bot] left review comments

@justinchuby justinchuby Awaiting requested review from justinchuby

At least 1 approving review is required to merge this pull request.

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
@jchen351 @snnn @tianleiwu @justinchuby