-
Notifications
You must be signed in to change notification settings - Fork 92
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
Organize #229
Organize #229
Conversation
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
This is really nice! Thank you for doing this. It was definitely easier to just look at the new branch to see the organization. I actually haven't looked at the diff yet, I think there are some code changes here and there that I'll check, but first, a handful of suggestions:
I think the Linux shell script is wrong here: https://github.com/shrit/examples/blob/organize/.ci/linux-steps.yaml#L99 --- it only changes up one directory (which used to be correct) but now it could be more than one, and has to go back to the root directory of the repository. Let me know what you think of the suggestions; don't feel obligated to take them all 😄 |
I agree with these suggestions, initially, I had it as
Will do this for sure.
I thought about this a lot, for the rain classification and examples that contain multiple techniques I did not find the best solution yet. However, I am 100% convinced that we need to separate them per method since most of our users will be looking for an example regarding a specific method. Also, most of our examples are named after datasets which itself does not make sense. Therefore, I really think that we need to class them per method.
Thanks for pointing this out, I started fixing it and I need to continue
|
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
Signed-off-by: Omar Shrit <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about this a lot, for the rain classification and examples that contain multiple techniques I did not find the best solution yet. However, I am 100% convinced that we need to separate them per method since most of our users will be looking for an example regarding a specific method. Also, most of our examples are named after datasets which itself does not make sense. Therefore, I really think that we need to class them per method.
This is going to be more interesting when it comes to embedded because we can show that some methods use fewer resources than others, or show examples of specific methods that can perform faster on low-resource devices.
Yeah, I agree with how the users will approach the examples. And it is probably true that many users will just poke into the directory structure to see what they are interested in. The question is just what to do with the multi-method examples. The best I can think of---and maybe you can think of something better---is just a multi-technique/
directory or something like this, and then an associated README that has quick descriptions of the examples and links between them. So e.g. you can put the "rain classification" example in the 'random forest' and 'logistic regression' sections in the README.
The perfect is the enemy of the good, so I agree it would be good to merge this even if we don't work out all the CI issues right now, and then come back for incremental improvements later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Second approval provided automatically after 24 hours. 👍
The aim of this PR is to make this repo usable.
The current state of this repo is a bit of a mess, this is the first attempt to organize examples based on the language and based on the ml method that is used.