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

Refactor Makefile to include venv creation and dependency installation #112

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

adarshan-intel
Copy link
Contributor

@adarshan-intel adarshan-intel commented Jan 9, 2025

Description:
This commit refactors the pytorch example Makefile to automate the creation of a virtual environment (venv) and the installation of necessary dependencies. The changes ensure that the environment setup is streamlined and dependencies are consistently installed without user intervention.

Changes:

  • Makefile:

    • Added steps to create and activate a virtual environment.
    • Included commands to install torchvision and pillow packages.
    • Updated the all target to depend on the venv creation.
    • Modified the distclean target to remove the virtual environment directory.
  • README.md:

    • Removed manual steps for creating a virtual environment and installing dependencies.

This change is Reviewable

@adarshan-intel
Copy link
Contributor Author

@kailun-qin @woju @mkow Can you review?

Copy link
Member

@woju woju left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @adarshan-intel)


pytorch/Makefile line 9 at r1 (raw file):

THIS_DIR := $(dir $(lastword $(MAKEFILE_LIST)))
VENV_DIR ?= $(THIS_DIR)/my_venv

If you're creating this virtual environment in the script, you probably want to add it to .gitignore. IIUC it wasn't there before, because it was supposed to be created (and removed) by the user.

Copy link
Contributor Author

@adarshan-intel adarshan-intel left a comment

Choose a reason for hiding this comment

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

Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @woju)


pytorch/Makefile line 9 at r1 (raw file):

Previously, woju (Wojtek Porczyk) wrote…

If you're creating this virtual environment in the script, you probably want to add it to .gitignore. IIUC it wasn't there before, because it was supposed to be created (and removed) by the user.

Done.
Good catch.

Copy link
Member

@woju woju left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL), "fixup! " found in commit messages' one-liners

Copy link
Contributor

@kailun-qin kailun-qin left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, all discussions resolved, "fixup! " found in commit messages' one-liners

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.

3 participants