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

Replace inline pip install with pip install from requirements*.txt #21106

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

jchen351
Copy link
Contributor

@jchen351 jchen351 commented Jun 19, 2024

Description

Replace inline pip install with pip install from requirements*.txt

Motivation and Context

so that CG can recognize

Dependency

tools/ci_build/build.py Fixed Show fixed Hide fixed
tools/ci_build/build.py Outdated Show resolved Hide resolved
wheel
pytest
numpy==1.24.0 ; python_version < '3.12'
numpy==1.26.0 ; python_version >= '3.12'
Copy link
Member

Choose a reason for hiding this comment

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

I have a PR(#21085) that updates Numpy to a newer version. Could you wait it? I think we have conflicts here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

update to numpy >= 1.19.0, same as your PR.

dep_packages.append("cerberus")
dep_packages.append("psutil")
run_subprocess([sys.executable, "-m", "pip", "install", *dep_packages])
def install_python_deps(requirements_file='requirements.txt',numpy_version=None):
Copy link
Member

Choose a reason for hiding this comment

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

Or maybe we can totally remove this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we want to keep the NumPy version overwritten capability? We can remove the function unless we don't want this capability.

Copy link
Member

Choose a reason for hiding this comment

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

I think we no longer need to.

Copy link
Member

Choose a reason for hiding this comment

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

We could just use the latest numpy

@@ -260,9 +260,6 @@ def convert_arg_line_to_args(self, arg_line):
"--wheel_name_suffix",
help="Suffix to append to created wheel names. This value is currently only used for nightly builds.",
)
parser.add_argument(
Copy link
Contributor Author

@jchen351 jchen351 Jun 20, 2024

Choose a reason for hiding this comment

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

The only place that uses this argument was from line 2813, which is no longer needed.

@jchen351 jchen351 marked this pull request as draft June 20, 2024 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants