Skip to content

Dash svm#101

Merged
shammamah-zz merged 85 commits intomasterfrom
dash-svm
Jul 30, 2019
Merged

Dash svm#101
shammamah-zz merged 85 commits intomasterfrom
dash-svm

Conversation

@YunkeXiao
Copy link
Copy Markdown
Contributor

@YunkeXiao YunkeXiao commented May 23, 2019

Issue for app: #39

App pull request

  • This is a new app
  • I am improving an existing app (redesigns/code "makeovers")

About

Workflow

  • I have created a branch in the appropriate monorepo, and the
    elements necessary for successful deployment are in place.
  • If the app is a redesigned and/or restyled version of an
    existing gallery app, I've summarized the changes requested in the
    appropriate Streambed issue and confirm that they have been applied.
  • If the app is on the Dash Gallery portal, I have added a link to
    the GitHub repository for the source code in the portal description.
  • If the app is a reimplementation of a Python gallery app for the
    DashR gallery, the app in this PR mimics, as closely as possible,
    the style and functionality of the existing app.

The pre-review review

I have addressed all of the following questions:

  • Does everything in my code serve some purpose? (I have removed
    any dead and/or irrelevant code.)
  • Does everything in my code have a clear purpose? (My code is
    readable and, where it isn't, it has been commented appropriately.)]
  • Am I reinventing the wheel? (I have used appropriate packages to
    lessen the volume of code that needs to be maintained.)

@YunkeXiao YunkeXiao requested review from ptr0x00 and shammamah-zz May 23, 2019 16:46
Copy link
Copy Markdown
Contributor

@shammamah-zz shammamah-zz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please finish up the mobile layout and re-request a review!

Comment thread apps/dash-svm/CONTRIBUTING.md Outdated
Comment thread apps/dash-svm/LICENSE.md
Comment thread apps/dash-svm/README.md Outdated
@@ -0,0 +1,82 @@
# Support Vector Machine (SVM) Explorer [![GitHub license](https://img.shields.io/github/license/plotly/dash-svm.svg)](https://github.com/plotly/dash-svm/blob/master/LICENSE.md) [![Mentioned in Awesome Machine Learning](https://awesome.re/mentioned-badge.svg)](https://github.com/josephmisiti/awesome-machine-learning)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Support Vector Machine (SVM) Explorer [![GitHub license](https://img.shields.io/github/license/plotly/dash-svm.svg)](https://github.com/plotly/dash-svm/blob/master/LICENSE.md) [![Mentioned in Awesome Machine Learning](https://awesome.re/mentioned-badge.svg)](https://github.com/josephmisiti/awesome-machine-learning)
# Support Vector Machine (SVM) Explorer [![Mentioned in Awesome Machine Learning](https://awesome.re/mentioned-badge.svg)](https://github.com/josephmisiti/awesome-machine-learning)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@YunkeXiao You didn't actually make this change, please commit the suggestion above.

Comment thread apps/dash-svm/requirements.txt Outdated
Comment thread apps/dash-svm/requirements.txt Outdated
@YunkeXiao YunkeXiao requested a review from shammamah-zz May 28, 2019 20:37
Copy link
Copy Markdown
Contributor

@shammamah-zz shammamah-zz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm getting some weird errors:

Screen Shot 2019-05-30 at 1 21 39 PM

The graph is also not showing up, and the mobile layout loads on desktop for some reason. Please fix these!

@YunkeXiao YunkeXiao requested a review from shammamah-zz July 19, 2019 20:58
Comment thread apps/dash-svm/assets/custom-styles.css Outdated
@@ -1,5 +1,7 @@
@import url('https://fonts.googleapis.com/css?family=Playfair+Display');

/* Body
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can get rid of this comment.

@YunkeXiao YunkeXiao requested a review from shammamah-zz July 23, 2019 18:55
@YunkeXiao YunkeXiao requested a review from celinehuang July 30, 2019 17:53
shammamah-zz
shammamah-zz previously approved these changes Jul 30, 2019
Copy link
Copy Markdown
Contributor

@shammamah-zz shammamah-zz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💃 Nice work!

Copy link
Copy Markdown
Contributor

@celinehuang celinehuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Super clean code 😃

Comment thread apps/dash-svm/README.md Outdated

Clone the git repo, then install the requirements with pip
```
git clone https://github.com/plotly/dash-svm.git
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please update to the new link (monorepo link)!

Comment thread apps/dash-svm/README.md Outdated
Clone the git repo, then install the requirements with pip
```
git clone https://github.com/plotly/dash-svm.git
cd dash-svm
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cd dash-sample-apps/apps/dash-svm

Comment thread apps/dash-svm/assets/custom-styles.css Outdated

/* Remove Undo
–––––––––––––––––––––––––––––––––––––––––––––––––– */
._dash-undo-redo {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this since the new version of Dash does this for us!

Copy link
Copy Markdown
Contributor

@shammamah-zz shammamah-zz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💃

Copy link
Copy Markdown
Contributor

@celinehuang celinehuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💃 woohoo looks good!

@shammamah-zz shammamah-zz merged commit 9935191 into master Jul 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants