-
Notifications
You must be signed in to change notification settings - Fork 32
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
Reorder README sections and print dataframe #95
Conversation
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.
Thank you very much for your contribution! I have some minor comments, the only one I consider critical is the link that directs to scikit-learn.
Hi @sgbaird, also from my side: thank you very much for all your input (especially the detailed feedback you gave in the issue you created). To proceed with this PR, I've pushed some changes on top, fixing the issues raised by my colleagues and also slightly adjusting the installation part a bit further (e.g. the extra dependencies are not strictly needed for the user guide example). Let me know if you have any further comments. Otherwise, I'll merge by the end of the day. If you encounter more things you prefer to have changed, we can always open another PR 👍🏼 |
@AVHopp @Scienfitz, could you please re-check and approve if OK? |
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.
Just found one missing word.
@sgbaird: would you like to put your name and affiliation to CONTRIBUTORS.md? Can wait with the merge ... |
You're welcome! Thank you for reviewing the changes and making edits 😊 Also, I updated the contributors file, but feel free to adjust. It's clear to me there has been a lot of work into making this a high-quality tool! I will try to loop back to the other items soon |
I think it's better to get to the quick start sooner, and leave the more in-depth installation options towards the end
Great, thanks for adding your name. Will merge now 🚀 |
I think it's better to get to the quick start sooner, and leave the more in-depth installation options towards the end.
I also think it might be worth using code formatting for the
df
output. For some reason, when I initially went through the docs I was under the impression there weren't any outputs (this was the case when I ran the code in Colab).