nidhaloff / igel

a delightful machine learning tool that allows you to train, test, and use models without writing code
https://igel.readthedocs.io/en/latest/
MIT License
3.09k stars 172 forks source link

re-write the cli using click (or maybe typer?) #63

Closed nidhaloff closed 3 years ago

nidhaloff commented 3 years ago

Description

I'm the creator and only maintainer of the project at the moment. I'm working on adding new features and thus I would like to let this issue open for newcomers who want to contribute to the project.

Basically, I wrote the cli using argparse since it is part of the standard language already. However, I'm starting to rethink this choice because it has some issues that the click library already overcome.

With that said, it would be great to re-write the cli in click or even in typer, which also uses click under the hood but adds more features.

If someone wants to work on this, please feel free to start directly, you don't need to ask for permission.

PS: Feel free to suggest other libraries. I just suggested click since I'm familiar with it

I hope not but If this issue stayed open for a long time, then I will start working on it myself

rmallof commented 3 years ago

Hi. Adding my 2 cents: maybe https://github.com/google/python-fire can help speed up the process :)

RackReaver commented 3 years ago

I use docopt for most of my projects now as click is usually overkill for what I need.

Based on what I see I think click is probably going to be your best option for your project as it has great modularity.

nidhaloff commented 3 years ago

@rmallof @RackReaver I'm not familiar with the two packages but thanks for the suggestion.

Yes I think click is a good choice for igel. Maybe typer would be even better but I'm not sure. I must check it out but it is also based on click and has similar syntax so that's the advantage

RackReaver commented 3 years ago

I'm not super familiar with ML but I'd be willing to give converting this a whirl. Do you have existing tests for the cli to ensure nothing is missed?

nidhaloff commented 3 years ago

@RackReaver At the moment there are no tests for the cli, but even then, it must be updated/adapted to the click syntax. I implemented the first simple version using argparse, which had limitations and this is exactly one of the reasons why I want to switch to click.

If you want to give it a try, I would happily review and eventually update your PR. You don't have to be familiar with ML, the cli code has nothing to do with ML, it's just python.

shubham3174 commented 3 years ago

Hi @nidhaloff, I am new to open source and would like to start working on it. I would try to replace argparse with click in cli.py

akshaynarisetti commented 3 years ago

@nidhaloff Could you please assign the issue under me, I have begun working on rewriting the code.

kuspia commented 3 years ago

@nidhaloff can u please share what were the issues you faced with argparse and why we need to rewrite it in click maybe I can fix that in argparse itself.

nidhaloff commented 3 years ago

@akshaynarisetti I don't have to assign you the issue. Just work on it and make a PR. Many people told me in the past that they started working on it, but I still did not receive any PR from them, so I will not assign the issue to anyone. Just make a PR when you are done.

@kuspia there are no issues with argparse. Click is just convenient to use, the code would be cleaner. I also like the click group feature, where you can group many sub commands under one parent command https://click.palletsprojects.com/en/1.x-maintenance/api.html#click.option If you have something in mind that you can improve using argparse, then create an issue for it and make a PR ;)

GouravWadhwa commented 3 years ago

Hi @nidhaloff, I have wrote something for command line arguments using click. I am still not sure how to integrate it and would really appreciate some suggestions. You can access the code from here https://github.com/GouravWadhwa/igel/blob/master/igel/click_cli.py.

kuspia commented 3 years ago

Hi @nidhaloff kindly check #89 it really enhances the cli.py using click version, I was working on it, if you will consider it, it will be my pleasure.