Pip package creation

[SkalskiP]

🛠️ PR Summary

_{Made with ❤️ by Ultralytics Actions}

🌟 Summary

Enhancements to Ultralytics' YOLOv5 Docker, GitHub CI workflows, and Python compatibility checks.

📊 Key Changes

• 📁 Refactored the Python package structure to accommodate setup.py for easier installs and imports.
• 🐍 Expanded Python version testing (3.7, 3.8, 3.9) in CI workflows for improved compatibility checks.
• 📦 Introduced a new CI workflow specifically for CPU-based testing across different Python versions.
• 🛠 Git-ignore rules and Docker ignore rules were updated to reflect the new package structure.
• 🐳 Docker build improvements, adjusting paths within the container.
• ✅ The addition of setup.py makes it possible to install YOLOv5 as a local package using pip install -e ..

🎯 Purpose & Impact

• These changes are geared towards making YOLOv5 more versatile and developer-friendly:
  • Easier to check for compatibility with multiple Python versions.
  • Simplifying the process of deploying on various environments with Docker.
  • Facilitating the ease of local development and testing through package installation.
• Users can look forward to a more robust, versatile tool, with streamlined contributions and deployment processes.
• Contributors can enjoy an improved testing suite that covers a wider range of scenarios.

[glenn-jocher]

@sheromon hi I wanted to invite you to contribute to this pip package PR. I know you made some good progress in #2886, but we wanted to start a new PR with a few changes. I'd like to get you on the author list for the change as well though since you did some great work in #2886!

One issue we're having is deserializing the currently saved models (saved with the current dir structure) when loading them in the new PR structure. Is this a problem you had in your PR?

[sheromon]

Haha, thanks, @glenn-jocher. I'm not worried about getting credit, I just thought it could be helpful to have the yolov5 namespace. Unfortunately, I don't have any datapoints that would be helpful. I didn't cross-use models created with code using two different directory structures, if that makes sense. I think that's what you're talking about.

[fcakyon]

@SkalskiP yolov5_in_syspath contextmanager is a direct copy from my repo: https://github.com/fcakyon/yolov5-pip/blob/8249d6188f40cbe709167c37f2f6a063ad5d6c2f/yolov5/utils/general.py#L696

you could have at least give credit to the original author :)

[fcakyon]

@glenn-jocher i have fixed that issue with yolov5_in_syspath contextmanager: https://github.com/fcakyon/yolov5-pip/blob/8249d6188f40cbe709167c37f2f6a063ad5d6c2f/yolov5/utils/general.py#L696

[sheromon]

FWIW, in my ideal world, what's in @fcakyon's yolov5-pip repo would get combined with what's in ultralytics/yolov5, but if there are issues with that (and I think that's what I heard), then oh well. Also in my ideal world, I can get in shape by eating Nutella crepes for breakfast, lunch, and dinner, so yeah.

[SkalskiP]

Hi @fcakyon I'm very sorry that you felt that way. This is just a PR draft for now, where we are trying to consider potential solutions and test them. It is very possible that the target solution will be different. But I have added credits and a link to the original repository.

And yes @sheromon and @fcakyon. We are concerned about models that were serialized in the old directory structure but will now be deserialized in the new one. For some reason, we need to use it like that:

def attempt_load(weights, map_location=None, inplace=True):    
    with yolov5_in_syspath():        
        from models.yolo import Detect, Model

So we actually need to import from models.yolo import Detect, Model not from yolov5.models.yolo import Detect, Model and that it is slightly worrying.

[glenn-jocher]

@SkalskiP I looked through this a bit. detect.py fails because our example images directory was deleted:
Exception: ERROR: /Users/glennjocher/PycharmProjects/yolov5/yolov5/data/images does not exist

Do you know of a way to revert the two deleted files? If not we may need to start a new PR, otherwise the git package will store the newly added files separately than the deleted ones, growing our git download size which we don't want.

EDIT: Once I replace the deleted images detect.py works again:

EDIT2: I see bus.jpg is pretty large, 473kb, I wonder if we might want to pass it through tinyjpg. Inference results may be slightly worsened, but the filesize would reduce significantly. Unfortunately the git package would only grow though as mentioned before... but the pip package would be reduced right?

[glenn-jocher]

@SkalskiP if I comment out the with statement and change imports to:
from yolov5.models.yolo import Detect, Model

I don't get a serialization error, I get a model difference error:
AttributeError: 'Detect' object has no attribute 'inplace'

This is due to the PR Detect and Model modules having different types than the v5.0 release checkpoints in https://github.com/ultralytics/yolov5/releases/tag/v5.0

One option for fixing this would be load and re-save all official v5.0 models in a new v5.1 release. detect.py has this checkpoint update capability with the --update flag:

python detect.py --update

yolov5/detect.py

Lines 179 to 182 in ba6f3f9

	if opt.update: # update all models (to fix SourceChangeWarning)
	for opt.weights in ['yolov5s.pt', 'yolov5m.pt', 'yolov5l.pt', 'yolov5x.pt']:
	detect(opt=opt)
	strip_optimizer(opt.weights)

[glenn-jocher]

@SkalskiP I've cleaned up the PR a bit, removed some un-needed changes, and applied a few fixes. Everything seems to work correctly now with the sys path fix being required in only one location (attempt_load() in experimental.py).

I tested train, test, detect, hubconf, all work. It looks like we have significant conflicts built-up over the week of master updates though, and we need to figure out how best to revert the data/images directory deletion if possible.

EDIT: Perhaps rather than fix the conflicts would it make more sense to start a new PR with the same changes, retaining the data/images directory?

[SkalskiP]

Thanks, @glenn-jocher I'll take a look at those changes right now. Just from reading your comments:

• data directory was not deleted, it was moved inside ytolov5 directory.
• let me try to fix those conflicts

[fcakyon]

@SkalskiP to entry points to work, you need to copy all changes from #3382

[SkalskiP]

@fcakyon doing that right now :) btw @fcakyon those are very good proposals <3

[fcakyon]

@SkalskiP i have a bit of experience with pip packaging, glad to be helpful here :)

[SkalskiP]

@glenn-jocher and @fcakyon my opinion is:

1. That we should not suggest users to install our project using requirements.txt. They should use setup.py. If they'll use pip package then great they use setup.py by default. If they clone repo they should do:

git clone https://github.com/ultralytics/yolov5
pip install -e .

2. I added script entry points suggested by @fcakyon. But even without them, there should not be a problem with running them both from yolov5 and yolov5/yolov5. There are ways to do it without sys.path.append, which - in my opinion - is unacceptable. Right now users can do it like that:

# from yolov5
python -m yolov5.detect --source yolov5/data/images/bus.jpg

# from yolov5/yolov5
python -m detect --source data/images/bus.jpg

# from yolov5
yolov5_detect --source yolov5/data/images/bus.jpg

# from yolov5/yolov5
yolov5_detect --source data/images/bus.jpg

# from yolov5
python yolov5/detect.py --source yolov5/data/images/bus.jpg

# from yolov5/yolov5
python detect.py --source data/images/bus.jpg

pip install -e . does much more than just install dependencies. For example, it does all sys operation for you.

[SkalskiP]

@glenn-jocher I cleaned up and removed those sys.path.append as well as is_pip function. All in all, we have support for 3.6.2-3.9 and we can run our script from multiple locations.

[SkalskiP]

@glenn-jocher, @fcakyon what do you thank? Are there are any more changes we should do? :)

[glenn-jocher]

@SkalskiP I see what you say in #3357 (comment), but in the real world users are going to do what they like, not what we tell them, so they will pip install -r requirements.txt as they've always done and then proceed directly to trying to run detect.py etc., which will shortly be followed by them raising a bug report when they see errors they don't understand, and then I'll have to spend my time explaining these breaking changes one by one to everyone:

[SkalskiP]

1. I merged develop into feature/restructuring_yolov5_codebase_to_prepare_for_pip_package_creation so we are up to date with changes
2. @glenn-jocher I realize that you are the creator of this repository and can only advise you. But my opinion is as follows:

• Remove requirements.txt from the repository and stick to setup.py. requirements.txt is redundant and will create confusion. (no requirements.txt -> no pip install -r requirements.txt)
• Update README.md with proper instructions.
• Stay away from sys.path.append if it's not necessary. (serialization - necessary; people do not know how to use python - not necessary)

[glenn-jocher]

@SkalskiP @fcakyon thanks for the updates guys!

I'm not sure what the best technical approach is here, but in terms of UX, we require minimization of breaking changes as an absolute priority. Remember we're not launching a new product here, we're updating a mid-lifecycle product, so this is not the appropriate moment for drastic changes to well established user workflows.

Keep in mind there are countless tutorials and videos on YouTube, Medium, Reddit, etc. about how to train and deploy YOLOv5 using universally established python norms like pip install -r requirements.txt and running simple commands like python train.py, so I'm very adverse to changes that would render all of these tutorials outdated or incorrect suddenly.

Our demographic also skews heavily towards novices in the field who may just be getting started with python, we don't want to create any barriers to entry or possible pain points, instead we want things to "just work" as Jobs used to say.

[glenn-jocher]

@SkalskiP @fcakyon maybe another option is to push as many updates as makes sense (such as #3382 for train, test, detect, export) to develop to better align the repo with the pip package requirements without actually taking the final step.

This might allow for easier maintenance for @fcakyon and give us some time to explore minimal-impact solutions with the least breaking changes. Then once we find the best solution the final pip PR would be trivial or at least less complicated.

[SkalskiP]

@glenn-jocher I understand all your concerns. However, in my opinion, pip package code distribution is an obvious step forward. This would allow many users to incorporate yolov5 into their projects much more easily.

Giving up this significant improvement for such (in my opinion) minor reasons as adding an extra level of directory depth and potentially (but not mandatorily) a different installation method is a mistake. As you know, yolov5 has quite a bit of technology debt, and if we don't have the ability and space to change anything in the future for similar reasons as we have today, it's hard to see how we can implement the plans we discussed.

I also do not quite understand the "just work" argument, as using setup.py basically guarantees stuff will work.

Regardless, if you want to take the "baby steps", then I would say, keep requirements.txt, use sys.path.append hack but don't cancel the pip feature all the way.

Let me know what you decided :)

[SkalskiP]

I took the time and I looked through some open source repositories and here is what I found:

• PyTorchLightning/pytorch-lightning use setup.py + requirements.txt
• qubvel/segmentation_models use setup.py + requirements.txt
• huggingface/transformers use setup.py
• huggingface/datasets use setup.py
• facebookresearch/detectron2 use setup.py

I found more examples, but I think these few already illustrate quite well that pip package is a must-have and certainly a step in the right direction.

[SkalskiP]

@glenn-jocher:

• I added the latest changes from develop
• I have implemented the changes we discussed. We have a separate CI for the scenario with installation via requirements.txt and setup.py. If you can take a peek and let me know if you have any more concerns.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions YOLOv5 🚀 and Vision AI ⭐.