Skip to content

[PyProject] Spin up an initial pyproject.toml allowing for local pip install #1527

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

Merged
merged 4 commits into from
Apr 16, 2025

Conversation

Jack-Khuu
Copy link
Contributor

@Jack-Khuu Jack-Khuu commented Apr 15, 2025

This is the first step towards pip-installing of torchchat. Specifically this PR:

  • Adds a pyproject.toml based on requirements.txt
  • Pulls torch installation instructions from install_requirements.sh into install_torch.sh
    • torch installation will be done separately from pip install

Note that install instructions have not changed pending refactoring the following from install_requirements.sh

(
  set -x
  $PIP_EXECUTABLE install evaluate=="0.4.3" lm-eval=="0.4.7" psutil=="6.0.0"
)

Testing:

# Original
./install/install_requirements.sh; pip freeze > original.txt

# New Install Requirements
./install/install_requirements.sh; pip freeze > new.txt

> diff original.txt new.txt 
None

@Jack-Khuu Jack-Khuu added triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module Installing Related to installing torchchat labels Apr 15, 2025
Copy link

pytorch-bot bot commented Apr 15, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/torchchat/1527

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit fbe31ff with merge base bf05236 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Apr 15, 2025
Copy link
Contributor

@zhenyan-zhang-meta zhenyan-zhang-meta left a comment

Choose a reason for hiding this comment

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

Awesome work! Left some nit and a question.

pyproject.toml Outdated
"tomli>=1.1.0; python_version < '3.11'",
"openai",
"wheel",
"cmake>=3.24,<4.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can keep the comments as we have in requirements.txt.

Comment on lines -94 to -98
#
# First install requirements in install/requirements.txt. Older torch may be
# installed from the dependency of other models. It will be overridden by
# newer version of torch nightly installed later in this script.
#
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: keep this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Intentionally dropped the extra index since it's not actually a case that's being used (i.e. difference in version requirements based on pt version)

fi
fi
echo "Using python executable: $PYTHON_EXECUTABLE"

Copy link
Contributor

Choose a reason for hiding this comment

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

qq: why do we not need to copy over

PYTHON_SYS_VERSION="$($PYTHON_EXECUTABLE -c "import sys; print(f'{sys.version_info.major}.{sys.version_info.minor}')")"
# Check python version. Expect at least 3.10.x
if ! $PYTHON_EXECUTABLE -c "
import sys
if sys.version_info < (3, 10):
sys.exit(1)
";
then
echo "Python version must be at least 3.10.x. Detected version: $PYTHON_SYS_VERSION"
exit 1
fi
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.

This version check is actually going to be removed once we move to pyproject (enforces python version), so if anything it will be deleted

Co-authored-by: Zhenyan Zhang (Meta) <[email protected]>
@zhenyan-zhang-meta
Copy link
Contributor

LGTM, feel free to merge.

@Jack-Khuu Jack-Khuu merged commit 3e348a2 into main Apr 16, 2025
72 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot. Installing Related to installing torchchat triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants