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

make installable pip package #465

Closed
wants to merge 15 commits into from
Closed

Conversation

Borda
Copy link
Contributor

@Borda Borda commented Jul 21, 2020

Reaction to #426 (comment)

TODO list:

  • wrap it to a package
  • add manifest
  • add setup
  • ci: tests package
  • auto/manual push to pip
    python setup.py sdist bdist_wheel
    twine upload ./dist/*
    

πŸ› οΈ PR Summary

Made with ❀️ by Ultralytics Actions

🌟 Summary

Enhanced CI testing capabilities and package installation for yolov5.

πŸ“Š Key Changes

  • Expanded Python version testing to include Python 3.7 alongside 3.8.
  • Modified data download path to reflect a new directory structure.
  • Added PyTest for code quality checks and coverage reporting.
  • Updated GitHub Actions workflow with additional checks for package metadata and dry run installation.
  • Included a manifest (MANIFEST.in) specifying project files for source distribution.
  • Created setup.py for PyPI package distribution, enabling pip install.
  • Restructured project directories, moving source files into a yolo5 subdirectory.
  • Updated README.md to include new installation instructions via pip.

🎯 Purpose & Impact

  • 🎨 Improves testing for multiple versions of Python, increasing compatibility.
  • πŸ› οΈ Ensures code quality and coverage with PyTest, leading to more reliable software.
  • πŸ“¦ Allows easy distribution and installation of yolov5 as a package, widening access to the software.
  • πŸ“ˆ Improves package metadata to comply with distribution standards.
  • πŸ—‚οΈ Restructuring creates a cleaner directory hierarchy, better separating source code from other project files.

@Borda Borda mentioned this pull request Jul 21, 2020
@Borda
Copy link
Contributor Author

Borda commented Jul 21, 2020

#426 (comment)

Borda I wanted to give you an update on v2.0. Most of the parts are in place, I'm waiting for a new yolov5x to finish training. It's at 190/300 epochs, training about 1 epoch/hr means it will be done roughly on Friday. After this I should be all set to merge a PR with the updated models and code changes, and then to write up a 2.0 release shortly afterwards.

What would be the steps then for compiling pip packages for the 2.0 version?

I have summarised the steps in todo for this PR

Do we need to register the repo or a user with the pip people to have them host our packages?

After these changes, anyone can install this package directly from GitHub as pip install git+https://github.com/ultralytics/yolov5.git or you can upload to PyPI... for that you need to create own/company account

Do we create 3 separate packages for the 3 main OS's?

need to check, but as there is no compiled or os depending code we shall be fine just with one package, but you should not worry about it as python setuptools does this for you

I've updated your CI code by the way to run tests on all 3 OS's and they are all currently passing now, so we appear to currently enjoy a minimum level of error-free compatibility across operating systems.

Great, I was adding also test fro python 3.7 as it is still widely used...

@Borda Borda force-pushed the add-setup branch 2 times, most recently from cd67786 to b755c7f Compare July 21, 2020 10:25
@Borda Borda marked this pull request as ready for review July 21, 2020 10:39
@Borda
Copy link
Contributor Author

Borda commented Jul 21, 2020

@glenn-jocher For installation, you can try:
pip install https://github.com/Borda/yolov5/archive/add-setup.zip
later the same will be on pip

@Borda
Copy link
Contributor Author

Borda commented Jul 21, 2020

to be pushed to PyPI you just run

python setup.py sdist bdist_wheel
twine upload ./dist/*

but be aware that once you upload a version of a package you cannot ever upload the same version...

@Borda
Copy link
Contributor Author

Borda commented Jul 21, 2020

It shall be also possible to call it from anywhere like this

python -m yolo5.train --arguments ... 

@glenn-jocher
Copy link
Member

@Borda thanks a lot for the PR! This looks really good, though is a bit more significant rework of the repo than I was expecting. Do we really need to drop all of files into a new (yolo5) directory as you've done here?

@Borda
Copy link
Contributor Author

Borda commented Jul 21, 2020

Borda thanks a lot for the PR! This looks really good, though is a bit more significant rework of the repo than I was expecting. Do we really need to drop all of files into a new (yolo5) directory as you've done here?

to be as one package yes, otherwise it appears in your site-package appears as independent models or utils which can be easily overwritten by any other packages installed after...

EDIT: the other work was using explicit imports so it is clear what is used where... using * was kind of black magic

@Borda Borda force-pushed the add-setup branch 2 times, most recently from 5d63187 to c9bf2e5 Compare July 22, 2020 20:34
@Borda
Copy link
Contributor Author

Borda commented Jul 22, 2020

@glenn-jocher so shall we move to the package? just asking as it is a quite a change each push to master makes it as collision and keeping it alive could be time-consuming...

@glenn-jocher
Copy link
Member

@Borda thank you! I'm worried about the scale of the change with nesting all of the files inside a yolo5 folder. I did not realize the pip package would require this sort of transformation of the repo. I think it's nice having many of the main files and folders at the top level as it makes it easier to gauge the repo at a glance for newcomers. I would say we should hold off on this for the time being, but I do appreciate your effort.

@Borda
Copy link
Contributor Author

Borda commented Jul 23, 2020

I understand that it is a quite hard decision... the call is if you prefer to have it (as it is now) just as a set of scripts which would accessible only with git-clone repo or rather a package which can be installed and allow users to use some other functions/modules/tolls you have developed so far ... 🐰

@Borda
Copy link
Contributor Author

Borda commented Aug 2, 2020

@glenn-jocher how about this one, or shall we close it? 🐰

@glenn-jocher
Copy link
Member

@Borda yes, I think let's close this one for now. Thanks!

@Borda Borda closed this Aug 2, 2020
@glenn-jocher glenn-jocher changed the title make installable package make installable pip package Sep 21, 2020
@glenn-jocher
Copy link
Member

@Borda I think the repo may finally be ready for a pip package soon. Your idea was a bit ahead of its time, but now its making more sense. We've completed PyTorch Hub updates that allow for super easy inference and results visualization, and I'm hoping that these may be recycleable when importing as a pip package. You can see these here:
https://pytorch.org/hub/ultralytics_yolov5/

import torch
from PIL import Image

# Model
model = torch.hub.load('ultralytics/yolov5', 'yolov5s', pretrained=True).autoshape()  # for PIL/cv2/np inputs and NMS

# Images
img = Image.open('zidane.jpg')  # PIL image

# Inference
results = model(img, size=640)  # includes NMS

# Results
results.show()  # .show() results, .save() jpgs, or .print() to screen

One cause for concern I had though was that the current repo would need to be nested inside another directory, making the repo a bit harder to sort through at first glance, and all of the bash commands a bit more lengthy. I'd recently learned about git submodules, that allow your github repo to contain another github repo as a submodule. Do you think this might make it possible for a seperate, dedicated pip package repo to contain ultralytics/yolov5 as a submodule?

You've done some great work here before, so any other ideas you have are always welcome!

@Borda
Copy link
Contributor Author

Borda commented Nov 24, 2020

@glenn-jocher I do not think that using sub-modules does not make sense in this case as in such case you need to create a new repo and also the new "parent-git-module" needs regular head updates which would mean some maintenance over time and confusion for users as there are two reports with the very same Yolo version...
So it is more like a move from a research project to a production package I think that the only reasonable way is refactoring and packaging 🐰 , in the end, you may find it even cleaner then it is now :]

@glenn-jocher
Copy link
Member

glenn-jocher commented Nov 27, 2020

@Borda hmm ok thanks for the advice. Yes I suppose you are right, the transition is scaring me a bit I guess. I'll sleep on it a bit more.

Perhaps another option would be to wait for github pypi packages support. I read this was coming eventually but apparently github is not in much of a hurry to implement this, so I think the timeline was around early-mid 2021.

@Borda
Copy link
Contributor Author

Borda commented Nov 27, 2020

@Borda hmm ok thanks for the advice. Yes I suppose you are right, the transition is scaring me a bit I guess. I'll sleep on it a bit more.

You can make this packaging in your personal fork and see how it is...

Perhaps another option would be to wait for github pypi packages support. I read this was coming eventually but apparently github is not in much of a hurry to implement this, so I think the timeline was around early-mid 2021.

what kind of support are you talking about? I guess that you always need to prepare a package with setuptools

@glenn-jocher
Copy link
Member

@Borda I'd noticed this issue: github/roadmap#94

And a thread here: https://github.community/t/pypi-compatible-github-package-registry/14615

But it seem we'd still need the same directory structure and setup.py in this case anyway.

@austin1howard
Copy link

I'm curious if any movement has been made on this front since November? I'm also desiring to use this repo's code for prod inference, as near as I can tell, autoshape and TTA aren't supported in the torchscript exported model. At the moment, I'm including your top-level models and utils directories in my code as well (to ensure that unpickling doesn't fail) which is less than ideal...

@glenn-jocher
Copy link
Member

glenn-jocher commented Apr 6, 2021

@austin1howard there is a 3rd party pip package (https://pypi.org/project/yolov5/) which attempts to replicate the current PyTorch Hub model behavior, though it is not guaranteed merged with master, so probably at the moment the best inference solution for python environments is simply to load your YOLOv5 model with PyTorch Hub. No code or imports are required other than import torch.

import torch

# Model
model = torch.hub.load('ultralytics/yolov5', 'yolov5s')
# model = torch.hub.load('ultralytics/yolov5', 'custom', 'best.pt')  # custom model

# Images
dir = 'https://github.com/ultralytics/yolov5/raw/master/data/images/'
imgs = [dir + f for f in ('zidane.jpg', 'bus.jpg')]  # batch of images

# Inference
results = model(imgs)
results.print()  # or .show(), .save()

See PyTorch Hub tutorial for more details.

YOLOv5 Tutorials

@austin1howard
Copy link

austin1howard commented Apr 6, 2021

ok thanks. Yeah I'd come across that particular project but wasn't sure how much (if at all) it was associated with this project. The PT Hub version will support TTA and autoshape? I'm also using a custom model, not the OOB one, but it looks like custom weights can be supported. EDIT: also looks like TTA and autoshape are supported too. I'll give it a shot. Thank you!

@Borda Borda mentioned this pull request Apr 23, 2021
@Borda Borda mentioned this pull request Dec 7, 2021
2 tasks
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

3 participants