New UI for prep stage #345

kirtansakariya · 2018-12-03T23:44:43Z

This addresses issue #336.

A GUI has been implemented using asciimatics to replace the old command prompt/terminal based prep stage.

Note: need to install asciimatics using pip install --src . -e git+https://github.com/peterbrittain/asciimatics.git#egg=asciimatics

… issues/336

…into issues/336

coveralls · 2018-12-04T20:42:06Z

Coverage decreased (-16.9%) to 57.681% when pulling 8618c3b on issues/336 into 0f0567b on master.

coveralls · 2018-12-04T20:42:07Z

Coverage decreased (-17.8%) to 56.875% when pulling d470a71 on issues/336 into d7378e9 on master.

JaimieMurdock

Hi Kirtan, a few comments on the revised filter functions - mostly do a refactor to raise ValueErrors on invalid responses rather than returning a valid = False variable.

JaimieMurdock · 2018-12-10T23:58:35Z

appveyor.yml

@@ -17,6 +17,7 @@ install:
  - "%PYTHON%\\python.exe -m conda install -q --yes cython scikit-learn pandas" # for vsm 
  - "%PYTHON%\\python.exe -c \"import nltk; nltk.download('stopwords'); nltk.download('punkt')\""
  - "%PYTHON%\\python.exe -m pip install unittest2 nose wget"
+  - "%PYTHON%\\python.exe -m pip install -e git+https://github.com/peterbrittain/asciimatics.git@fcedb4947933de7e1507ec0dee8ca7a3f466928a#egg=asciimatics"


is there a particular reason why we need that specific commit for asciimatics? Will it be folded into a release?

As of right now, the version of asciimatics that we need has not had an official release. I asked them when they plan on doing another release and they said that it would be in a couple of months. So until them, we will have to use that specific commit or the asiimatics repo in general.

JaimieMurdock · 2018-12-11T00:00:56Z

requirements.txt

@@ -1,3 +1,4 @@
+-e git+https://github.com/peterbrittain/asciimatics.git@fcedb4947933de7e1507ec0dee8ca7a3f466928a#egg=asciimatics


Again, do we need the specific commit? Opening it in edit mode also introduces a dependency on git. This is not desired as it makes non-CS student installs way more complicated (install anaconda, install git, change the path, install topicexplorer, ...)

Same reason as above.

JaimieMurdock · 2018-12-11T00:27:19Z

tests/test_prep.py

@@ -58,50 +58,54 @@ def test_get_candidate_words():

 @patch('topicexplorer.prep.input')
 def test_get_high_filter(input_mock):


There will be some revisions to these tests based on changes to the topicexplorer.prep.get_high_filter_stops call signature - namely, it should not return a third part of the tuple called valid - rather a ValueError exception should be raised and handled by the caller.

JaimieMurdock · 2018-12-11T00:28:49Z

topicexplorer/prep.py

+    import numpy as np
+    input_filter = num
+    valid = True
+    f = open("test.txt", "w+")


This test code should be removed.

JaimieMurdock · 2018-12-11T00:29:50Z

topicexplorer/prep.py

+        filtered += u' '.join(candidates)
+
+        if len(candidates) == len(c.words):
+            valid = False


raise a ValueError here, rather than returning an invalid state. Our style should favor exceptions that are explicitly handled, rather than letting it silently fail and introduce inconsistency later.

JaimieMurdock · 2018-12-11T00:32:47Z

topicexplorer/prep.py


+
+        if len(candidates) == len(c.words):
+            valid = False


again, introduce a ValueError that can be try-except-ed rather than a new variable to test.

kirtansakariya · 2018-12-18T21:31:10Z

I'm raising the ValueError as requested now. As for pulling the asciimatics code, I still have it as copying the GitHub repo. I'm not too sure if there's another alternative. Should I try downloading it not in edit mode by removing the -e flag?

JaimieMurdock · 2018-12-30T04:15:01Z

The lack of a published patch is quite the problem for the software to end users, as opposed to developers. We do not want to require our users to set up git in order to install topicexplorer.

there are a few options:

Wait for asciimatics to mint a new release.
Release a fork of asciimatics specific for our project to PyPI, then change deps once the new release is minted from asciimatics.
Delay release of the new interface to the public until the asciimatics release is done.

@kirtansakariya, can you provide any links or info about the release timeline from your conversations with the upstream developers?

kirtansakariya · 2018-12-31T01:45:06Z

I'll leave a log of the conversations that we had, separated by which issues they were addressing

Text widget max length
peterbrittain/asciimatics#182
peterbrittain/asciimatics#184

FileBrowser filtering and pull request that talks about when the next release is going to be
peterbrittain/asciimatics#186
peterbrittain/asciimatics#187

Label height
peterbrittain/asciimatics#193

JaimieMurdock · 2020-04-29T04:21:26Z

Hey @kirtansakariya, it looks like all of your patches made it into v1.11.0. With some testing I think we can merge this!

Going to start this tonight:

make sure that features work with out-of-the-box asciimatics 1.11.0
merge master into this branch to update all new changes.
get @colinallen, @kirtansakariya and @JaimieMurdock to review

JaimieMurdock · 2020-04-29T04:59:06Z

So I'm having some stalling issues with a low frequency filter of 20% on the AP corpus. This is rather small... List finally printed after 3 minutes, but isn't scrollable.

It also seems like Ctrl+C isn't working to terminate the program - perhaps asciimatics captures this?

kirtansakariya · 2020-04-29T18:28:16Z

Back when I was working on it I don't think I had the option to make the list scrollable but I can look into it again to see if they've added anything like that yet or if I had just missed it last year.

kirtansakariya · 2020-05-05T19:51:50Z

I'm thinking this might be related to issue #351.
I may try to look into issue #351 and see if that ends up fixing this one.

kirtansakariya added 18 commits October 3, 2018 22:05

done with skeleton for asciimatics

de39576

copy before removing comments

79f7db4

removed comments

562df93

removing booleans meant for highlighting fields

4f8b799

added comments

07c39d3

removed most unused variables

4789197

making text fields on summary smaller

9299811

need to handle file selection in file browser

0cc848d

handling invalid values for percentages

fe80ac1

adding comments

d3964bc

file preview, rearranging wizard screens

3c7f493

Merge branch 'master' into issues/336

b8abbff

adding asciimatics to the requirements.txt

6af97af

Merge branch 'master' of https://github.com/inpho/topic-explorer into…

79e51aa

… issues/336

Merge branch 'issues/336' of https://github.com/inpho/topic-explorer …

4637720

…into issues/336

demo works now

2ffed42

working on tests

bb38671

modifying test cases

8618c3b

kirtansakariya added 2 commits December 5, 2018 14:34

adding asciimatics install to windows build

7dcde53

adding dividers for wizard screens

6d43236

JaimieMurdock requested changes Dec 11, 2018

View reviewed changes

raising ValueError now

7b7ede8

using instance of unittest.TestCase

af695ed

kirtansakariya added 2 commits December 30, 2018 20:45

Merge branch 'master' into issues/336

327d4c5

Merge branch 'master' into issues/336

0dbab9c

kirtansakariya added 2 commits February 23, 2019 14:36

Merge branch 'master' into issues/336

a970384

Merge branch 'master' into issues/336

7dc3ff7

updating asciimatics reqs

7751cfe

interrupts enabled

d470a71

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New UI for prep stage #345

New UI for prep stage #345

kirtansakariya commented Dec 3, 2018

coveralls commented Dec 4, 2018

coveralls commented Dec 4, 2018 •

edited

Loading

JaimieMurdock left a comment

JaimieMurdock Dec 10, 2018

kirtansakariya Dec 11, 2018

JaimieMurdock Dec 11, 2018

kirtansakariya Dec 11, 2018

JaimieMurdock Dec 11, 2018

JaimieMurdock Dec 11, 2018

JaimieMurdock Dec 11, 2018

JaimieMurdock Dec 11, 2018

kirtansakariya commented Dec 18, 2018

JaimieMurdock commented Dec 30, 2018

kirtansakariya commented Dec 31, 2018

JaimieMurdock commented Apr 29, 2020 •

edited

Loading

JaimieMurdock commented Apr 29, 2020 •

edited

Loading

kirtansakariya commented Apr 29, 2020

kirtansakariya commented May 5, 2020

		@@ -1,3 +1,4 @@
		-e git+https://github.com/peterbrittain/asciimatics.git@fcedb4947933de7e1507ec0dee8ca7a3f466928a#egg=asciimatics

		@@ -58,50 +58,54 @@ def test_get_candidate_words():

		@patch('topicexplorer.prep.input')
		def test_get_high_filter(input_mock):

New UI for prep stage #345

Are you sure you want to change the base?

New UI for prep stage #345

Conversation

kirtansakariya commented Dec 3, 2018

coveralls commented Dec 4, 2018

coveralls commented Dec 4, 2018 • edited Loading

JaimieMurdock left a 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

kirtansakariya commented Dec 18, 2018

JaimieMurdock commented Dec 30, 2018

kirtansakariya commented Dec 31, 2018

JaimieMurdock commented Apr 29, 2020 • edited Loading

JaimieMurdock commented Apr 29, 2020 • edited Loading

kirtansakariya commented Apr 29, 2020

kirtansakariya commented May 5, 2020

coveralls commented Dec 4, 2018 •

edited

Loading

JaimieMurdock commented Apr 29, 2020 •

edited

Loading

JaimieMurdock commented Apr 29, 2020 •

edited

Loading