-
-
Notifications
You must be signed in to change notification settings - Fork 69
remove ultralytics dependency #262
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @lachiewalker, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello team,
Gemini here, providing a summary of this pull request authored by @lachiewalker. The main objective of this PR is to remove the direct dependency on the ultralytics
package from the yolov5-pip
distribution. This is achieved by copying the specific utility functions from ultralytics.utils
that were being used directly into the yolov5/utils/general.py
file. The motivation behind this change, as stated in the description, is to reduce the overall package size, which is particularly beneficial for deployments in environments like AWS Lambda, and to pave the way for more granular, optional dependencies in the future (e.g., yolov5[train]
, yolov5[inference]
, etc.). The author emphasizes that this change is intended to be completely backwards compatible with no alteration to existing APIs or behavior.
Highlights
- Dependency Removal: The direct dependency on the
ultralytics
package has been removed from therequirements.txt
file. - Code In-lining: Necessary utility functions previously imported from
ultralytics.utils
have been copied directly intoyolov5/utils/general.py
. These functions include checks for online status, retries, requirement parsing, and version checking. - Package Size Reduction: A primary goal of this change is to decrease the size of the
yolov5-pip
package by removing the largeultralytics
dependency. - Preparation for Modular Dependencies: This PR is described as a foundational step towards introducing optional, sub-module dependencies in future releases.
- No Behavior Change: The author asserts that this change maintains full backwards compatibility and does not alter the functionality or APIs of the package.
Changelog
- requirements.txt
- Removed the
ultralytics>=8.0.100
entry (line 18).
- Removed the
- yolov5/utils/general.py
- Added imports for
functools
(line 7),importlib.metadata
(line 24), andtypes.SimpleNamespace
(line 30). - Removed the import
from ultralytics.utils.checks import check_requirements
(line 39). - Added a large block of code (lines 127-433) containing in-lined utility functions (
is_online
,Retry
,parse_requirements
,parse_version
,check_uv
,check_version
,check_requirements
) copied from theultralytics
repository, including license attribution comments.
- Added imports for
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Dependency gone,
Copied code now stands alone,
Package size shrinks down.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request effectively removes the ultralytics
dependency by vendoring the required utility functions. The motivation for reducing package size and preparing for sub-module packaging is well-explained. The changes are mostly straightforward, involving copying code and updating requirements.txt
.
I've identified one critical issue related to Python version compatibility that needs to be addressed and one medium-severity security consideration for the copied code. Otherwise, the PR achieves its stated goal.
Summary of Findings
- Python Version Incompatibility: The code uses the walrus operator (:=), which is Python 3.8+, but
setup.py
claims Python 3.7+ support. This will break for Python 3.7 users. - Potential Command Injection Risk: The copied
attempt_install
function usessubprocess.check_output
withshell=True
, which can be a security risk if input variables are not properly sanitized.
Merge Readiness
The pull request makes good progress towards decoupling from the ultralytics
library. However, there is a critical Python version compatibility issue due to the use of the walrus operator, which will prevent users on Python 3.7 from using this package. This issue must be addressed before merging.
Additionally, there's a medium-severity security concern regarding the use of shell=True
in the copied subprocess
calls, which should be considered for future improvement.
I am not authorized to approve pull requests, but I recommend that the critical compatibility issue be resolved before this PR is merged. Other reviewers should also assess the changes.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Pinned |
requirements.txt
Outdated
@@ -12,10 +12,9 @@ PyYAML>=5.3.1 | |||
requests>=2.23.0 | |||
scipy>=1.4.1 | |||
thop>=0.1.1 # FLOPs computation | |||
torch>=1.7.0 # see https://pytorch.org/get-started/locally (recommended) | |||
torchvision>=0.8.1 | |||
torch==2.5.1 # see https://pytorch.org/get-started/locally (recommended) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this was done because torch 2.6.0+ has weights_only
defaulting to True?
Instead of locking the torch version, it's better to add weights_only=False
parameter to torch.load()
calls, and bump minimal version to torch>=2.0.0
(where this option was first introduced).
Or just set env variable TORCH_FORCE_NO_WEIGHTS_ONLY_LOAD=yes
in CI -- this would work with any PyTorch release without bumping the min version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although I wanted this change to be minimal, and not alter minimum version requirements for dependencies, changing the CI as you proposed could result in bugs where the CI tests only pass because the TORCH_FORCE_NO_WEIGHTS_ONLY_LOAD=yes
env var is present, whilst users who set up an environment with torch>=2.0.0
will see different behaviour due to the changed default of torch.load(weights_only=False->True)
. I thus opted to add weights_only=False
to all torch.load()
calls because, despite raising the minimum version for key dependencies, the changed logic is explicit.
Thanks for your continued feedback!
…hvision (min. req. ver. = 2.0.0)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes the direct dependency on ultralytics by in-lining the required utility functions and updating torch.load calls to include the weights_only parameter.
- In-lined several utility functions from ultralytics into the repo (e.g., check_requirements, parse_requirements, check_version).
- Updated torch.load calls in multiple modules to explicitly pass weights_only=False.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
yolov5/utils/general.py | In-lined ultralytics utilities and updated module imports for in-lined code. |
yolov5/train.py | Updated torch.load call to include weights_only=False to load checkpoints correctly. |
yolov5/segment/train.py | Similar torch.load update as in train.py for segmentation models. |
yolov5/models/experimental.py | Updated torch.load call for ensemble model loading. |
yolov5/hubconf.py | Updated torch.load call to use weights_only=False when loading weights. |
Comments suppressed due to low confidence (5)
yolov5/utils/general.py:141
- The os module is used here but is not imported. Add 'import os' at the top of the file.
assert str(os.getenv("YOLO_OFFLINE", "")).lower() != "true" # check if ENV var YOLO_OFFLINE="True"
yolov5/utils/general.py:196
- The time module is used for sleeping but is not imported. Please add 'import time' at the top.
time.sleep(self.delay * (2**self._attempts)) # exponential backoff delay
yolov5/utils/general.py:152
- The platform module is used here but is not imported; ensure to add 'import platform' at the top of the file.
ARM64 = platform.machine() in {"arm64", "aarch64"} # ARM64 booleans
yolov5/utils/general.py:227
- The re module is used for regex matching but is not imported; please add 'import re' to the beginning of the file.
match = re.match(r"([a-zA-Z0-9-_]+)\s*([<>!=~]+.*)?", line)
yolov5/utils/general.py:256
- The subprocess module is used to run shell commands but is not imported; add 'import subprocess' to the file.
return subprocess.run(["uv", "-V"], capture_output=True).returncode == 0
This change removes the direct dependency on
ultralytics
from the yolov5-pip package by in-lining the utility functions (and the handful of utils they depend on from the same ultralytics.utils module) which were previously imported fromultralytics.utils
. No other changes were made to the codebase.Summary of Changes
ultralytics>=8.0.100
entry inrequirements.txt
.Impact & Compatibility
No behavior change: All functions and APIs remain identical. This change is completely backwards compatible.
Motivation & Next Steps
This is an incremental cleanup to shrink package size as I think the yolov5 package is very well written for training, inference, and model export but there is currently no way to opt out of GPU and training dependencies. As deploying onto AWS lambda functions is a frequent use case of mine, this is very inefficient from a storage and container image size perspective. This decoupling from
ultralytics
is intended as the precursor to a future separate PR which will introduce sub-module packaging (e.g.yolov5[train]
,yolov5[inference]
, etc.). For now, this patch focuses solely on removingultralytics
as a dependency without altering any APIs.