[MRG] Added Jensen Shannon metric and dendrogram visualization #1484

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

docs/notebooks/Topic_dendrogram.ipynb

+   "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

docs/notebooks/Topic_dendrogram.ipynb

+    "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

docs/notebooks/Topic_dendrogram.ipynb

+   "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

docs/notebooks/Topic_dendrogram.ipynb

+    "\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

docs/notebooks/Topic_dendrogram.ipynb

+    "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

docs/notebooks/Topic_dendrogram.ipynb

+    "# 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

docs/notebooks/Topic_dendrogram.ipynb

+    "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

docs/notebooks/Topic_dendrogram.ipynb

+    "                                  '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

docs/notebooks/Topic_dendrogram.ipynb

+    "                                  '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

docs/notebooks/Topic_dendrogram.ipynb

+    "                                   '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

docs/notebooks/Topic_dendrogram.ipynb

+    "# 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MRG] Added Jensen Shannon metric and dendrogram visualization #1484

[MRG] Added Jensen Shannon metric and dendrogram visualization #1484

parulsethi commented Jul 13, 2017 •

edited

Loading

piskvorky commented Jul 19, 2017

piskvorky commented Jul 21, 2017

parulsethi commented Jul 24, 2017

piskvorky commented Jul 24, 2017

parulsethi commented Jul 25, 2017

piskvorky commented Aug 3, 2017 •

edited

Loading

parulsethi commented Aug 3, 2017

piskvorky commented Aug 4, 2017 •

edited

Loading

parulsethi commented Aug 4, 2017 •

edited

Loading

menshikh-iv commented Aug 22, 2017

menshikh-iv Aug 22, 2017

menshikh-iv Aug 22, 2017

menshikh-iv Aug 22, 2017

menshikh-iv Aug 22, 2017

menshikh-iv Aug 22, 2017

menshikh-iv Aug 22, 2017

menshikh-iv Aug 22, 2017

menshikh-iv Aug 22, 2017

menshikh-iv Aug 22, 2017

menshikh-iv Aug 22, 2017

menshikh-iv commented Aug 22, 2017

piskvorky left a comment

piskvorky Aug 22, 2017

menshikh-iv commented Aug 23, 2017

[MRG] Added Jensen Shannon metric and dendrogram visualization #1484

[MRG] Added Jensen Shannon metric and dendrogram visualization #1484

Conversation

parulsethi commented Jul 13, 2017 • edited Loading

piskvorky commented Jul 19, 2017

piskvorky commented Jul 21, 2017

parulsethi commented Jul 24, 2017

piskvorky commented Jul 24, 2017

parulsethi commented Jul 25, 2017

piskvorky commented Aug 3, 2017 • edited Loading

parulsethi commented Aug 3, 2017

piskvorky commented Aug 4, 2017 • edited Loading

parulsethi commented Aug 4, 2017 • edited Loading

menshikh-iv commented Aug 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

menshikh-iv commented Aug 22, 2017

piskvorky left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

menshikh-iv commented Aug 23, 2017

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