[MRG] Added Jensen Shannon metric and dendrogram visualization by parulsethi · Pull Request #1484 · piskvorky/gensim

parulsethi · 2017-07-13T17:59:52Z

Adds Jensen-Shannon distance metric and topic dendrogram-heatmap visualization notebook.

piskvorky · 2017-07-19T05:06:09Z

Hanging indent everywhere please -- vertical indent makes the code hard to read and hard to maintain.

piskvorky · 2017-07-21T13:08:19Z

@parulsethi It's Jensen-Shannon (after Johan Jensen and Claude Shannon).

…into jenson_shannon

parulsethi · 2017-07-24T11:13:17Z

@piskvorky Can you please point to which line should be changed to hanging indent? As if i understand correctly hanging/vertical indent is used in case of long arguments? and there weren't any in this PR so I didn't use any line breaks in code

piskvorky · 2017-07-24T11:49:45Z

Unfortunately Github says the notebook is too large and its diff cannot be displayed, so I cannot review directly. But the indentation is broken around annotation_html =, update( and a few other places.

There are also a few minor code style inconsistencies, like spaces around , or :. @menshikh-iv will direct you :)

@menshikh-iv is there a way to somehow review the notebook? I'd like to point out some constructs which, while not wrong, could be improved a little for better readability (like map(int(), to be more "Pythonic".

parulsethi · 2017-07-25T12:19:29Z

@piskvorky I'll remove the cell outputs once the notebook is complete. It will then have a much smaller diff containing only the input code cells which could be displayed in Github for code review.

piskvorky · 2017-08-03T12:22:24Z

@parulsethi let's fix that Jenson everywhere, before it propagates too far through the code/notebooks.

parulsethi · 2017-08-03T18:24:25Z

I've removed the plotly code and cell outputs for smaller diff on github, will add it back after the code review of notebook is complete. If needed, the output visualization can be seen here in a previous commit.

@piskvorky corrected the name in a notebook comment it was being used in. Elsewhere it's the function name.

piskvorky · 2017-08-04T07:43:35Z

The name is incorrect and needs to be fixed, that's what I mean.

parulsethi · 2017-08-04T14:10:47Z

Oh, the spelling. Sorry, misunderstood earlier

menshikh-iv · 2017-08-22T06:07:14Z

@piskvorky About notebook - nbdime good tool for view diffs between notebooks, but this tool doesn't integrate with github.

Also, we can use nbconvert, but this again does not suit us. Probably it makes sense to talk with tech GitHub support and discuss their plans to improve the work with notebooks.

menshikh-iv · 2017-08-22T06:14:29Z

+   "outputs": [],
+   "source": [
+    "from gensim.models.ldamodel import LdaModel\n",
+    "from gensim.corpora import Dictionary, MmCorpus\n",


Unused imports: gensim.corpora.MmCorpus, scipy.spatial.distance.squareform, plotly.figure_factory

menshikh-iv · 2017-08-22T06:16:16Z

+    "texts = []\n",
+    "for line in df_fake.text:\n",
+    "    lowered = line.lower()\n",
+    "    words = re.findall(r'\\w+', lowered, flags = re.UNICODE | re.LOCALE)\n",


Unnecessary spaces flags = re.UNICODE | re.LOCALE

menshikh-iv · 2017-08-22T06:17:29Z

+   "source": [
+    "from gensim.matutils import jensen_shannon\n",
+    "\n",
+    "from random import sample\n",


Unused imports: random.sample, scipy

menshikh-iv · 2017-08-22T06:19:06Z

+    "\n",
+    "# Plot dendrogram\n",
+    "dendro = create_dendrogram(topic_dist, distfun=js_dist, labels=range(1, 36), annotation=annotation)\n",
+    "dendro['layout'].update({'width':1000, 'height':600})\n",


Spaces after :

menshikh-iv · 2017-08-22T06:19:43Z

+    "annotation = text_annotation(topic_dist, topic_terms, n_ann_terms)\n",
+    "\n",
+    "# Initialize figure by creating upper dendrogram\n",
+    "figure = create_dendrogram(topic_dist, distfun=js_dist, labels = range(1, 36), annotation=annotation)\n",


no needed spaces labels = range(1, 36)

menshikh-iv · 2017-08-22T06:25:12Z

+    "# Add Heatmap Data to Figure\n",
+    "figure['data'].extend(heatmap)\n",
+    "\n",
+    "dendro_leaves = [x+1 for x in dendro_leaves]\n",


Missing whitespace x+1

menshikh-iv · 2017-08-22T06:25:47Z

+    "dendro_leaves = [x+1 for x in dendro_leaves]\n",
+    "\n",
+    "# Edit Layout\n",
+    "figure['layout'].update({'width':800, 'height':800,\n",


missing spaces after : (and below)

menshikh-iv · 2017-08-22T06:26:53Z

+    "                                  'showline': False,\n",
+    "                                  \"showticklabels\": True, \n",
+    "                                  \"tickmode\": \"array\",\n",
+    "                                  \"ticktext\" : dendro_leaves,\n",


Unnecessary space (before :) and below

menshikh-iv · 2017-08-22T06:27:02Z

+    "                                  'showline': False,\n",
+    "                                  \"showticklabels\": True, \n",
+    "                                  \"tickmode\": \"array\",\n",
+    "                                  \"ticktext\" : dendro_leaves,\n",


Unnecessary space (before :) and below

menshikh-iv · 2017-08-22T06:27:20Z

+    "                                   'showline': False,\n",
+    "                                   'zeroline': False,\n",
+    "                                   'showticklabels': False,\n",
+    "                                   'ticks':\"\"}})\n",


missing space

menshikh-iv · 2017-08-22T06:34:24Z

@parulsethi please fix PEP things, return all images and resolve merge conflict. LGTM for me

piskvorky

Typo.

piskvorky · 2017-08-22T06:45:42Z

+    "# no. of terms to display in annotation\n",
+    "n_ann_terms = 10\n",
+    "\n",
+    "# use Jenson-Shannon distance metric in dendrogram\n",


One more "Jenson".

menshikh-iv · 2017-08-23T06:42:24Z

Good job @parulsethi 🥇

* add notebook with js heatmap * separate out vector conversion function * modify notebook * add cluster heatmap notebook * print y-axis values * correct y labels * add nb details * few more nb details * add text labels * print linkage/dendo data * add annotations for all hierarchy levels * make diff annotations symmetric * add Plotly's dendrogram code * train for more passes * remove outputs * remove plotly code and imports * err.. re-run cells * fix jensen spelling * made requested changes

parulsethi added 6 commits July 13, 2017 17:11

add notebook with js heatmap

91b7eec

separate out vector conversion function

7a5c0c3

modify notebook

a190f7d

add cluster heatmap notebook

2fce748

print y-axis values

1614dfb

correct y labels

d6601e5

add nb details

30ad840

parulsethi added 2 commits July 22, 2017 22:26

Merge branch 'develop' of https://github.com/RaRe-Technologies/gensim …

21df1cb

…into jenson_shannon

few more nb details

f0ba371

piskvorky changed the title ~~Added Jenson shannon metric~~ [WIP] Added Jensen Shannon metric Jul 24, 2017

parulsethi added 2 commits July 25, 2017 15:13

add text labels

a702843

print linkage/dendo data

4998dfb

parulsethi added 2 commits August 2, 2017 05:59

add annotations for all hierarchy levels

b31badb

make diff annotations symmetric

c1e2b9b

parulsethi mentioned this pull request Aug 2, 2017

Add flag parameters for topic diff #1448

Merged

add Plotly's dendrogram code

2bfdbed

parulsethi added 4 commits August 3, 2017 23:19

train for more passes

b9e7ab5

remove outputs

b8be8db

remove plotly code and imports

ee2a8c1

err.. re-run cells

70b474a

fix jensen spelling

34308a7

menshikh-iv suggested changes Aug 22, 2017

View reviewed changes

piskvorky requested changes Aug 22, 2017

View reviewed changes

parulsethi added 2 commits August 23, 2017 09:20

made requested changes

cd8ced7

Merge branch 'develop' into jenson_shannon

159edb8

parulsethi changed the title ~~[WIP] Added Jensen Shannon metric~~ [MRG] Added Jensen Shannon metric Aug 23, 2017

parulsethi changed the title ~~[MRG] Added Jensen Shannon metric~~ [MRG] Added Jensen Shannon metric and dendrogram visualization Aug 23, 2017

menshikh-iv merged commit de3e2cb into piskvorky:develop Aug 23, 2017

parulsethi deleted the jenson_shannon branch August 23, 2017 10:37

Uh oh!

Conversation

parulsethi commented Jul 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

piskvorky commented Jul 19, 2017

Uh oh!

piskvorky commented Jul 21, 2017

Uh oh!

parulsethi commented Jul 24, 2017

Uh oh!

piskvorky commented Jul 24, 2017

Uh oh!

parulsethi commented Jul 25, 2017

Uh oh!

piskvorky commented Aug 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

parulsethi commented Aug 3, 2017

Uh oh!

piskvorky commented Aug 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

parulsethi commented Aug 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

menshikh-iv commented Aug 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

menshikh-iv commented Aug 22, 2017

Uh oh!

piskvorky left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

menshikh-iv commented Aug 23, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

parulsethi commented Jul 13, 2017 •

edited

Loading

piskvorky commented Aug 3, 2017 •

edited

Loading

piskvorky commented Aug 4, 2017 •

edited

Loading

parulsethi commented Aug 4, 2017 •

edited

Loading