Add CLI Fire for detection script

[Borda]

Resolves #4511

🛠️ PR Summary

_{Made with ❤️ by Ultralytics Actions}

🌟 Summary

Refactoring detect.py for improved maintainability and adding Google Fire CLI.

📊 Key Changes

• Removed argparse and replaced it with fire for parsing command line arguments.
• Changed the function signature of run to use type hints for better readability and static type checking.
• Added a function summary for run with descriptions of each argument.
• Removed parse_opt and main functions as they're replaced by the use of fire.
• Added Google Fire to the requirements.txt file for dependency installation.

🎯 Purpose & Impact

• Simplicity: Streamlines command-line interface creation, making it easier to build and use.
• Type Safety: Type hints help developers understand what types of arguments are expected and tools to perform type checking.
• Readability & Maintenance: Documentation for run arguments makes it clear what each parameter does, contributing to better code maintainability.
• User Experience: May enhance user experience with an interactive and more flexible command-line interface.
• Requirement Changes: Users need to install the new dependency (fire) to use the updated detect.py.

[Borda]

@glenn-jocher mind check? 🐰

[glenn-jocher]

@Borda this is really interesting. Didn't know about Sequence. Is Fire and Sequence compatible with python >= 3.6?

It looks like one downside is that the arguments must be full-length now. i.e. can we do python detect.py --img 640 or do we need to explicitly do python detect.py --imgsz 640 for Fire to work correctly?

[Borda]

Is Fire and Sequence compatible with python >= 3.6?

yes, so far I know, the Sequence comes from typing not from Fire

can we do python detect.py --img 640 or do we need to explicitly do python detect.py --imgsz 640 for Fire to work correctly?

I guess that the img de deprecated argument and kept only for back-compatibility reasons
in general, I would use the new imgsz as the past one was quite confusing what it does...

[Borda]

@glenn-jocher so how do you feel about this simification?

[Borda]

@glenn-jocher can we proceed this one, it is not trivial to keep it updated :]

[Borda]

@glenn-jocher ^^ 🐰

[Borda]

@glenn-jocher ^^ 😕

[glenn-jocher]

@Borda hey there! Thanks for submitting this and keeping it updated. Do you think this is suitable for merging into master or do you think we might want to run some more tests on this? Any significant change in the repo like this we want to be sure we are handling correctly.

If you're free next week for a quick meeting please use my meeting link to set up a call. Thanks!

[Borda]

@glenn-jocher I would say it is safe to merge as the detection quite straight script... for training or other, I would recommend running larger testing... 🐰

[Borda]

@glenn-jocher Happy New Year 🎉

[Borda]

@glenn-jocher still an ongoing discussion? 🐰

[chris-clem]

It would be great to have that feature!

[Borda]

@glenn-jocher ay update here? 😕 it is pending for almost half a year...

[Borda]

@glenn-jocher ^^

[glenn-jocher]

@Borda I've been testing this, so far so good, but we've lost the capability to print the arguments as in master on this line:

yolov5/detect.py

Line 241 in 932dc78

print_args(FILE.stem, opt)

yolov5/utils/general.py

Lines 164 to 167 in 932dc78

	def print_args(name, opt):
	# Print argparser arguments
	LOGGER.info(colorstr(f'{name}: ') + ', '.join(f'{k}={v}' for k, v in vars(opt).items()))

I've been looking at using locals() and vars() to recreate this, but can't separate out the function/Fire variables with existing variables created by the import statements at the top of the file.

[Borda]

I've been looking at using locals() and vars() to recreate this, but can't separate out the function/Fire variables with existing variables created by the import statements at the top of the file.

this simple example works:

import inspect
import fire


def main(a1: int, b2: str = "aaa"):
    myframe = inspect.currentframe()
    args, _, _, frm = inspect.getargvalues(myframe)
    args = {k: v for k, v in frm.items() if k in args}
    print(args)


if __name__ == '__main__':
    fire.Fire(main)

[Borda]

@glenn-jocher all seems fine :)

[glenn-jocher]

@Borda I ran into two issues when testing:

Shortened arguments

• Shortened arguments no longer work, i.e. --img is no longer supported, couldn't find an easy workaround. Many examples and tutorials use these commands, i.e. our Colab notebook.

Multiple weights

• Multiple --weights used for ensembling no longer work, i.e. --weights model1.pt model2.pt. This may just require changing the weights variable type to a list, but haven't tested yet. In master correct Usage is below (super convenient and understandable for tutorials):

[Borda]

Add the short args, yes it is a limitation
Add multiple args, you need to pass them as list or tuple

[Borda]

@glenn-jocher ^^ 🐰

[github-actions]

This pull request 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 ⭐.

[glenn-jocher]

@Borda thanks for the heads-up! We'll look into adding support for shortened arguments and accommodating multiple weights as lists or tuples. Appreciate your input!

[Borda]

It was a few years ago lol

[glenn-jocher]

@Borda time flies! Your past insights are still valuable as we continuously improve YOLOv5. Thank you for your contributions, both then and now! 😄