-
-
Notifications
You must be signed in to change notification settings - Fork 16.6k
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
Add yolov5 namespace #2886
Add yolov5 namespace #2886
Conversation
…fcakyon's yolov5-pip repository.
…statements as in yolov5-pip repo. Note: I had to make a few changes from what is in yolov5-pip because there appear to have been some code changes between the version in yolov5-pip and where I branched off of the yolov5 repo. While I was updating import statements, I noticed that there are still a couple of places in .py files where the import path isn't updated to include the yolov5 namespace in yolov5-pip, and I stayed consistent with what's in yolov5-pip by not updating those import statements.
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 @sheromon, thank you for submitting a 🚀 PR! To allow your work to be integrated as seamlessly as possible, we advise you to:
- ✅ Verify your PR is up-to-date with origin/master. If your PR is behind origin/master an automatic GitHub actions rebase may be attempted by including the /rebase command in a comment body, or by running the following code, replacing 'feature' with the name of your local branch:
git remote add upstream https://github.com/ultralytics/yolov5.git
git fetch upstream
git checkout feature # <----- replace 'feature' with local branch name
git rebase upstream/master
git push -u origin -f
- ✅ Verify all Continuous Integration (CI) checks are passing.
- ✅ Reduce changes to the absolute minimum required for your bug fix or feature addition. "It is not daily increase but daily decrease, hack away the unessential. The closer to the source, the less wastage there is." -Bruce Lee
@sheromon thanks for the PR! Users have made similar suggestions in the past, including @fcakyon and @Borda with an earlier PR. We'd like to strike a balance between exportability/deployability and training/development so we've avoided this so far, thinking it would complicate matters for many use-cases. Is there a middle-ground that might work so that someone could clone yolov5 within their own project and then use the same import structure you have above? Perhaps if we made the imports relative rather than absolute? |
@glenn-jocher im not 100% sure but relative imports might make it easier |
@fcakyon @sheromon, I'm not really sure either. I understand of course that dropping everything into a subfolder would make packaging easier, but it would also create more hurdles and possible failure points when new users were trying to learn and reproduce the tutorials, i.e. the current command is hard to fail, even if no arguments are passed we've designed it to run correctly the first time, and there's no ambiguity really about where to run it from: $ python train.py But under the proposed PR there would be 4 possible cases and probably many more bug reports from confused users in the future (especially first-time uses, which we really focus on): $ python train.py # works from yolov5/ but fails from .
$ python yolov5/train.py # works from . but fails from yolov5/ EDIT: if the git clone directory and package directory are identically named it might lead to further confusion also, i.e. user raises an issue saying "I ran this command from the yolov5/ directory and {bug here}" and we would not know what that meant. |
Check make installable pip package #465 |
@glenn-jocher, oh, I hadn't realized that this change had been proposed before. Now that you mention it, I can see the concern about confusion about which directory to run from. It seemed pretty clear to me, but I guess I'm somewhat used to working with Python code that is set up to be a pip package now. It's true that I don't have experience maintaining code for a large number of users -- I mainly write code for myself and my teammates, so I recognize that you have the much more experienced perspective. I feel like relative imports wouldn't work, because sometimes you'd need to include a parent directory in the import path, e.g. in Anyway, the motivation was to solve the import issues that I and some others had encountered. To fix my problem for just me, I could probably just work off of the yolov5-pip repo instead. Update: Oh! Maybe if this repo mentioned yolov5-pip? I just saw the part in #465 where someone said they'd found yolov5-pip but weren't sure "how much (if at all) it was associated with this project". It seems like it would be helpful to at least some people if maybe the ultralytics/yolov5 repo mentioned yolov5-pip so the relationship would be a little bit clearer. |
I retract my request |
The changes here to address #2481 move the yolov5 code into a yolov5 directory with an
__init__.py
file to makeyolov5
a Python package. When yolov5 is installed using pip, modules and functions can then be imported likefrom yolov5.utils.autoanchor import check_anchors
with no need to add any directories to the Python path. This is a problem that many users have encountered in the past as in #134 and #353.Adding
yolov5
to the import statement can also help to disambiguate in the event that a user has something else namedmodels
orutils
in their Python path.These changes are intentionally made to be consistent with changes made by @fcakyon in yolov5-pip and make the code in these two repos more similar.
A TODO item is updating the docs to reflect the new locations for various files. I thought about doing that but figured it made more sense to get some feedback on these changes first.
🛠️ PR Summary
Made with ❤️ by Ultralytics Actions
🌟 Summary
Reorganization of the YOLOv5 codebase with directory structure changes and updates to files for better modularity.
📊 Key Changes
yolov5
directory..vscode
to.gitignore
to prevent committing Visual Studio Code-specific files..gitignore
to match the new structure.🎯 Purpose & Impact