-
-
Notifications
You must be signed in to change notification settings - Fork 172
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
provide a basic GUI for users who prefer to uses GUI #17
Comments
I suggest that we (everyone who read this issue) add a command to trigger GUI, some things like |
well said. I like the idea of |
The GUI should have:
Note: Import, export .yaml files |
@dinhanhx I don't mind that ;) However, I was thinking that a yaml file will not be needed in the gui funtionality. Instead, the user can upload the data as csv (or maybe enter the path to the data), select the model he wants to use (from a select button with multiple items) and then click ok. This were my thoughts but It's flexible ;) |
Your idea is more user-friendly than mine |
@dinhanhx please tell me If you are working or want to work on this in order to clarify the state of this feature ;) |
@nidhaloff I want to work on this feature but I am quite busy with university work at the moment |
I could write something up for this using a basic tkinter UI, but there seem to be quite a few available options under read_data_options. Perhaps something like a button that allows you to add another data option from a dropdown? |
@peter-shoes Thanks for joining the project. Yes a drop-down menu was also what I thought about. I think that would be great, otherwise you can be creative as you want ;) |
@nidhaloff I started working by hardcoding the options, but it occurred to me that you might wanna update things in the future. I was able to find options for "type", "split", "preprocess", "model", and "target" in the code to pull from, but there is nothing referencing any of the "read_data_options". Should these just be hardcoded? |
@peter-shoes You mean default values? you are right. I provided default values for "split", "preprocess" etc.. but I didn't do that for read_data_options yet since it's a new feature that I implemented this week. The read_data_options are basically the arguments that will be given to the |
@nidhaloff just an update! i've been working on this and i think i'll just do the one pull request when i've completed it. i'm taking strings from the readme in order to get these massive lists of options for "model_type" and "read_data_options". should be done by the weekend probably? |
@peter-shoes Alright cool. Happy coding ;) |
@nidhaloff Howdy! i've completed the GUI with the exception of the options for "handle missing values" under preprocessing. Cant seem to find the other options besides the ones you've listed. Also, just making sure you know, the program will return a dictionary formatted the same way as the yaml file, and I'm not sure where to hand that off to. |
@peter-shoes Awesome! Thanks for the hard work. Can you show some screenshots or ideally gifs about it and which values can the user select in the UI? Here is an overview about the missing values options:
Great that a dictionary is returned, which will match the same yaml file format. I'm excited to see it, it would be great to upload some gifs or screenshots ;) |
@peter-shoes god work |
@peter-shoes is everything ok? |
@nidhaloff everything is okay! pushing final version today and i'll take some screenshots as well. sorry for going dark! been a busy week! |
@nidhaloff one final question: i have added functionality for selecting the actual file you want to use, because i thought that was what 'dataset:' meant. is this correct? also, for arguments under hyperparameter_search, should the gui take actual arguments there? other than cv, refit, etc. |
@peter-shoes Yes that is correct! Ideally, the user should be able to provide the hyperparameter search arguments. However, you don't need to implement everything. If you have some functionality, we can make a draft release of our gui just to show users what it is capable of. We can always improve that in the future. You have done a great work, I think we should see that and make a release so that you also get credits for what you ve done ;) You can extend this in the future. That would be great if you take this task and work on it whenever you have some time. Like for example, we can release the version you have now without hyperparameter search, but you can in parallel work on a version that will support hyperparameter search and we can release that when you are done. I think that would be more comfortable for you, right? |
@nidhaloff so i took out the hyperparameter search arguments, but it's still in a comment. I'm finally done! i'll do a PR as soon as i finish this message. images are below. file select screen on startup: my cursor is apparently not in any of these, but this was supposed to demonstrate how you have to confirm the algorithm type with the button before the proper algorithm menu appears: finally, adding grid options for hyperparameter search: Had a ton of fun doing this! thanks for letting me be a part of the team guys! and thanks for being so patient too! Best, |
@nidhaloff Also, once you accept the PR, would you label it "hacktoberfest-accepted"? i doubt i'll actually make my PR quota by the end of the month but it's worth a shot anyway. Additionally, i do intend to keep working on this because this version looks a little...rough. |
@peter-shoes Looks cool! sorry I couldn't reply yesterday, I'm very busy these days. The screenshots looks good, I will merge the PR now and check it out. I will also grant your wish and create a "hacktoberfest-accepted" label for your PR. Thank you for your work. This is undoubtedly the most important PR I received until now ;), so you need to be proud of what you achieved. I hope you will keep working on this and share the same vision & goal of the project, which is to bring ML to everyone. Your task or rather your work is IMO the most valuable, since non-technical users will love to use a GUI, even if using the igel CLI is actually pretty straightforward. As you can see, many people are giving us positive feedback and the project made it to daily as well as weekly trending, so many people will use your code/product in the future, it's definitely a good feeling. Keep it up and thanks for joining us ;) I will close the issue, but I recommend to open a new issue whenever you start working on some improvement for the GUI. For example, you can open an issue and label it as feature/enhancement if you want to add the hyperparameter search functionality in the UI. |
@peter-shoes I thought Tkinter comes with the standard library, now I'm getting important not found. Apparently, tkinter needs python-tk and not the normal version of python. I didn't know that --' maybe tkinter wasn't the best choice after all. what do you think? Maybe other people can join the discussion and give their opinion. |
@nidhaloff yeah, I was worried about that when i first started work. However, according to the official documentation, linked below, Tk comes with most binary distributions of python 3, though I myself had to install it separately. This is the closest thing to GUI implementation in the standard library, as far as i can tell. |
@peter-shoes Tkinter can be installed by installing python3-tk, which is not quite straightforward for the users and also for the development. See, I'm working in a virtual environment and I couldn't manage to create a virtualenv and install tkinter in it even though I installed the python3-tk distribution. Users who will install igel will have to also install this distribution where tkinter is supported, otherwise igel will not work properly on their machine. This is quite a catch, especially for non-technical users who will never figure this out and will eventually drop the idea of using igel. I thought tkinter comes with the standard library without installing any other package or distributions of python, that's why I suggested it. However, I'm starting to think that this is not the right choice. Do you have any other ideas? Or can you somehow investigate how to make the setup of tkinter easier so that users will not have to do this separately? Maybe we can update the readme and tell users to install that python distribution that comes with tkinter |
@peter-shoes I managed to install the python distribution that comes with tkinter and tried to run the gui script. However, I got this error:
I don't understand, what does README have in common with the GUI? Can you briefly give me an overview of what is happening exactly in the gui file? I didn't find comments that explain the logic. Did you got this to work on your machine? could you choose a csv file without a problem? |
@nidhaloff I actually had a dream about this the other night. This was a workaround I used to get the read_data_options and the available algorithms. Basically, the README is being parsed for those options, like it gets the options from the README. When I first started working on this I thought I was being clever, but later on i realized this was a pretty bad idea, because the readme can change at any time. The solution to this is to simply hardcode the options for both read_data_options and algorithms. I can do this today. |
@nidhaloff, as for the Tk problem, it would be pretty much impossible to code a GUI by hand without using another language or a website. Additionally, Tk code does not migrate well to other python based GUI packages. I think the best thing to do would be to include a small section in the README on how to install Tk, either with terminal examples or through conda or whatever windows people use. |
@peter-shoes Okay, yeah that's not a good idea since the readme will definitely change and keep on changing. You can even remove the whole read_data_options part for now. It's not a priority. Can you fix this and push to the igel gui branch instead of master? |
@nidhaloff yeah i'll get on this right away, again, my apologies |
@nidhaloff pushed to gui functionality branch of my fork |
@peter-shoes Thanks! I tried it now. I achieved to open it and choose a dataset. However, I noticed the window is not responsive and I couldn't click ok at the end to close the window and receive the dictionary. Is this not implemented yet or did I missed something? |
@nidhaloff the main function returns the dictionary when the window is closed. It does not print the dictionary. Should it print?Should I add a button to close the window instead? |
@peter-shoes Also FYI, I made some research and figure out that we can use the electron framework to build a desktop app that looks very good! with web languages (html, css, js). I thought it would be a great idea, so I made a repo for it and started the development. We can try to develop a desktop app with electron in parallel to this, since we talked that tkinter may not be the best choice. here is the repo of the UI project https://github.com/nidhaloff/igel-ui I appreciate it if you will consider to contribute to this project too, or maybe help promote it. |
@peter-shoes Yes it would be great to click OK at the end when I'm done and the window will close and the dictionary will be returned. |
@nidhaloff just pushed the update, the button closes the window and the resulting dictionary is both printed to console and returned by the function. I find it strange that you would want to implement a new GUI based in another language, likely with its own set of requirements. I cannot help you with this, as I have not written in JS for many years now, but I wish you the best of luck. |
@peter-shoes thanks! I still want to work on this of course. This tkinter GUI will be integrated in igel and not separate. Why do you find it strange that I want to implement a new GUI? We already noticed the problems of tkinter and hence I nice looking cross platform app is a good idea, right |
@nidhaloff from my experience, the more languages and package dependencies you add, the more difficult a piece of software becomes to maintain. That said, I do think a separate application might be useful if you can find the right person or people to contribute. |
@peter-shoes python is poor when it comes to GUI apps as you ve seen. Developing a GUI app in python is not a great idea. A basic gui in tkinter is OK, but as you can see the configurations are harder and the support is poor. Finding contributors who know web technologies is way easier than finding people who know how to program a gui in python ( since it's not a popular use case for python). Sure it will be difficult to maintain, but I'm actually certain that maintaining a tkinter app would be harder. As an example, if you will not work anymore on the tkinter app, then I must read and understand your code, which would mean that I need to understand the basics of tkinter first and then move to understanding your code. I'm a bad designer/frontend developer actually. Therefore, since we all familiar with web technologies, IMO it would be easier to understand/maintain html, css and simple js code from my side and furthermore, easier to find contributors who are familiar with these languages than a specific python library like tkinter. |
@nidhaloff that's reasonable. I will continue to maintain this code as long as it is necessary, just shoot me a message whenever something breaks or needs updating. |
@peter-shoes Thanks. There is already a lot that can be improved :D for instance, the window is not responsive. If I make it smaller, I wouldn't be able to see the other UI elements. |
@nidhaloff try it now, this is pretty good, but it's admittedly not perfect. It would be tough to do really beautiful responsiveness without massive restructuring. |
@peter-shoes I already tried. It's better than the other version but still not quite responsive. Also on the left, there is much space that it's not used and I couldn't select an algorithm to use. Proposed solution for now:
The important thing is to let it really simple to use. Most people will use the terminal commands. This gui functionality is useful for non-technical users and hence it's very important to provide a simple thing. Thanks :) |
@nidhaloff yeah the space on the left is so you can add as many read_data_options as you want, but I guess if we're getting rid of that I can just shift everything over. As for the rest of the stuff I can just comment it all out. Do you want to remove hyperparameter search as well? |
@peter-shoes Thanks. hmm depending on how the gui will look like. You can decide that. You can pretend you are a user and evaluate the GUI yourself 👍🏼 If you remove read_data_options and use the space on the left, it would be really great! |
@nidhaloff howdy! just did the changes in like 20 minutes, sorry i've been away for so long (kinda crazy in america right now)! So I commented out the features we talked about, and now it looks a lot more cut-and-dry, just a very very simple interface. I'm thinking I might clean it up from here? maybe just make it look like a proper UI. Also, it still returns the same dictionary, just with None values for anything the UI doesn't cover. Hope you've been well! |
Description
Users should have the flexibility of using a simple GUI if they don't want to use the CLI. A simple GUI can be made with Tkinter maybe?
It's important to not add any dependency for this. It would be better to implement this using an existing python module.
The text was updated successfully, but these errors were encountered: