Added function to replace gensim check_output with subprocess.check_output by kirit93 · Pull Request #1182 · piskvorky/gensim

kirit93 · 2017-03-04T06:54:34Z

@tmylk I've added a function above the existing check_output function in an attempt to solve this issue. Please let me know if there's anything I need to change. This is with reference to issue #703

tmylk · 2017-03-05T14:56:39Z

@kirit93 Thanks for the PR.
How do we know that the output is printed to the log? Please add a test

Could you please replace the actual check_output and run the tests for mallet, dtm, fasttext and wordrank wrappers.

There was some reason I added extra KeyboardInterrupt handling to the code - need to look more in order to find what was the issue now. Please check that behaviour is preserved.

kirit93 · 2017-03-06T18:37:15Z

@tmylk I ran the tests for mallet, dtm, fasttext and wordrank wrappers. All pass except for fasttext, but the error I get with my check_output function is the same I get when I use the existing gensim check_output function. So I'm not sure why that is failing, any idea?
This is what is shown on the log -

======================================================================
FAIL: testSimilarity (__main__.TestFastText)
Test similarity for in-vocab and out-of-vocab words
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_fasttext_wrapper.py", line 139, in testSimilarity
    self.assertEqual(self.test_model.similarity('night', 'nights'), self.test_model.similarity('nights', 'night'))
AssertionError: 0.3508754735782928 != 0.35087547357829274

----------------------------------------------------------------------
Ran 15 tests in 9.282s

FAILED (failures=1)

I'll get started with the rest of the things you mentioned.

piskvorky · 2017-03-06T19:17:36Z

Looks like a bad test -- floats should never be compared for bit equality. Instead, there's np.allclose() etc.

kirit93 · 2017-03-06T19:29:49Z

Should I fix those tests?
Currently the function uses :

self.assertEqual(self.test_model.similarity('night', 'nights'), self.test_model.similarity('nights', 'night'))

I'll change it to :

self.assertTrue(numpy.allclose(self.test_model.similarity('night', 'nights'), self.test_model.similarity('nights', 'night')))

I made the change and ran the test and there are no failures, the assertEqual is used in a couple of other places in this test. If this fix is okay, I'll make the changes everywhere.

tmylk · 2017-03-06T20:27:20Z

@kirit93 Confirm that using allclose is the right way to go.

Does the error output get printed to the log with this new change?

kirit93 · 2017-03-07T16:50:45Z

@tmylk numpy.testing.assert_allclose() seems to be a better option for float comparisons. It prints a verbose error message to the log unlike numpy.allclose. It uses a default relative tolerance of 1e-7 which can be set to 0 if desired.

tmylk

Please address comments about exception handling

tmylk · 2017-03-07T23:23:51Z

+        raise
+    except:
+        error = "Error in check_output while trying to execute: \n ' " + str(args) + " '\nthis command was not found"
+        print(error)


please output to the log

Is there a particular log file I should be logging output to? If not STDOUT, what is the right way to send out the error message? In the earlier function the output and error were being returned from the function. Should I do the same? I could output it to STDERR as the error generated by the shell when it tries executing the incorrect command is also written to STDERR.

tmylk · 2017-03-07T23:24:49Z

-        process.terminate()
+        raise
+    except:
+        error = "Error in check_output while trying to execute: \n ' " + str(args) + " '\nthis command was not found"


Could there be exceptions even when command is found? if yes, then please remove "command was not found"

Yes, I overlooked that detail. Should I change the error message to something more generic like " xyz command could not be executed "?

tmylk · 2017-03-07T23:25:25Z

+        return res
    except KeyboardInterrupt:
-        process.terminate()
+        raise


what is the purpose of catching and immediately throwing the exception? this line doesn't add anything as is

It makes sure the catch-all except: below doesn't apply to KeyboardInterrupt.

But it's not a good practice anyway, because there's also SystemExit etc. At least limit the catch-all to Exception.

Would it be a good idea to catch the subprocess.CalledProcessError and output an error message that is easy for the user to understand? This is the error raised when check_output fails.
For any other error including KeyboardInterrupt I can use a catch-all Exception and raise it.

piskvorky · 2017-03-09T04:33:56Z

+        return res
    except KeyboardInterrupt:
-        process.terminate()
+        raise


It makes sure the catch-all except: below doesn't apply to KeyboardInterrupt.

But it's not a good practice anyway, because there's also SystemExit etc. At least limit the catch-all to Exception.

piskvorky · 2017-03-09T04:34:45Z

     """
     selected_keys = random.sample(list(d), min(len(d), n)) if use_random else itertools.islice(iterkeys(d), n)
-     return [(key, d[key]) for key in selected_keys]
+     return [(key, d[key]) for key in selected_keys]


Unwanted change.

piskvorky · 2017-03-09T04:36:44Z

+        raise
+    except:
+        error = "Error in check_output while trying to execute: \n ' " + str(args) + " '\nthis command was not found"
+        print(error)


The original exception was more useful, because it included stdout/stderr, which useful for error debugging.

Is there any way to do the same here?

I'm sorry, I didn't fully understand what you mean here. Which original exception are you talking about?

I meant subprocess.CalledProcessError, a few lines above.

I don't have any strong opinion on what this function should do on failure, or how to capture the subprocess stdout/stderr. Except for sure we don't want to be printing anything -- logging preferred.

piskvorky · 2017-03-09T04:37:56Z

+    In case args generates an error
+    >>> test_checkoutput(args=['/usr/bin/pythons', '-ve']) #Incorrect argument
+    /bin/sh: /usr/bin/pythons: No such file or directory
+    *Error in args : /usr/bin/pythons -ve


Is this something that is printed? Or logged?

In general, we don't want to pollute stdout at all, unless the user explicitly requested printing.

The No such file or directory line gets outputted to STDERR as it is the error returned by the shell when it tries to execute the command in args.
The Error in args: /usr/bin/pythons -ve message gets printed to STDOUT because I'm using a simple print command. Is there a log file you'd like me to log the output to or should I print to STDERR if we don't want STDOUT polluted?

tmylk · 2017-03-19T01:08:34Z

+        Instead of raising the error, output a more specific error message
+        """
+        error = "subprocess.check_output could not execute command ' " + str(args) + " '"
+        print(error, file=sys.stderr)


Please log instead of printing, see examples in the code

Please output to log instead of printing.

tmylk

Please add tests for the exception handling

tmylk · 2017-03-19T01:10:00Z

+        error = "subprocess.check_output could not execute command ' " + str(args) + " '"
+        print(error, file=sys.stderr)
+        return error
+    except Exception:


this code is redundant. It will be raised anyway

tmylk · 2017-03-19T01:10:26Z

+            self.assertTrue(True)

+class TestOutput(unittest.TestCase):
+    def test_check_output(self):


Please add a test that exception is raised

kirit93 · 2017-03-19T05:22:39Z


+    def test_check_output_exception(self):
+        error = utils.check_output(args=['ldfs'])
+        self.assertEqual(error, "subprocess.check_output could not execute command ' ldfs '")


test_utils.py imports utils.py from the installed gensim. So this test case is still running the old version of check_output and therefore the exception test fails. However, if you change the import to use the current version of utils.check_output, all tests pass.

you can install your new version of gensim with python setup.py install

tmylk · 2017-03-23T00:31:43Z

Please add unit test that exception is indeed raised.

… replaced deprecated assertEquals with assertEqual

tmylk · 2017-03-28T13:14:16Z

+        """
+        error = "subprocess.check_output could not execute command ' " + str(args) + " '"
+        logger.error(error)
+        return error


what is the reason to return instead of previous raise?

The reason I've chosen to simple return the error message is because raise results in the following large and not user friendly message.

Traceback (most recent call last): File "test.py", line 51, in test_check_out res = subprocess.check_output(args, shell=flag) File "/Users/kirit/anaconda/lib/python3.5/subprocess.py", line 626, in check_output **kwargs).stdout File "/Users/kirit/anaconda/lib/python3.5/subprocess.py", line 708, in run output=stdout, stderr=stderr) subprocess.CalledProcessError: Command 'ldfs' returned non-zero exit status 127 During handling of the above exception, another exception occurred: Traceback (most recent call last): File "test.py", line 76, in <module> test_check_out(args=["ldfs"]) File "test.py", line 60, in test_check_out raise error TypeError: exceptions must derive from BaseException

Raising a string (instead of some exception) is definitely not a good idea.

How would we benefit from raising an exception here? Once the subprocess.CalledProcessError occurs and is caught, an appropriate message is returned to the user indicating why the check_output command failed. Wouldn't that be enough for the user to proceed correctly?

Your raise error already raises an exception. It's just that proper exceptions inherit from Exception (or appropriate subclasses), rather than being just strings.

I could do a raise Exception(error).
Would that be okay?

I haven't looked into the actual logic. Just commenting on the discussion above, where you say The reason I've chosen to simple return the error message is because raise results in the following large and not user friendly message., but that message is only due to raising a string instead of a proper exception :)

My 2c would be to use the same error-handling logic that the function employed previously, before your changes. The least surprise to users.

kirit93 · 2017-04-19T06:32:19Z

@tmylk @piskvorky I've modified the function to handle errors just as the earlier function would. I hope this is okay. Please let me know if there are any further changes to be made so that the PR can be merged. Thanks!

tmylk · 2017-05-02T19:23:03Z

+        """
+        error = "subprocess.check_output could not execute command ' " + str(args) + " '"
+        logger.error(error)
        raise


Please raise the CalledProcessError exception as before.

I may be missing something, but why exactly do we need to raise the CalledProcessError at all. As long as we tell the user why check_output failed, shouldn't that be fine? Using shell=true with subprocess.check_output, in case of erroneous input, the same error message gets displayed as it would if the user were to enter the command on the terminal. This serves the purpose of telling the user why check_output failed.

If I pass ldfs to check_output, stderr will have /bin/sh: ldfs: command not found. Similarly, if you were to type the same command on the terminal, the output would be bash: ldfs: command not found.

In the earlier code stderr would have had a huge error message indicating a FileNotFound. If required CalledProcessError can be captured and a similar error message can be logged.

In case I've missed something and there is some reason that an implementation similar to the earlier one is preferred, I will look into that.

First of all we need the actual output to be written to log, not to stderr.
Secondly, the tests should check that the return message is indeed for ldfs: command not found. The current message they test for is not informative

Would it be okay for the log to have the error message as follows

/bin/sh: ldfs: command not found Command '['ldfs']' returned non-zero exit status 127.

along with a subprocess.CalledProcessError being raised?

In the test case, I'm unable to figure out how to use assertRaises to verify that it was in fact a subprocess.CalledProcessError that was raised.

tmylk · 2017-05-02T19:30:10Z

It seems that tests didn't run for the last commit, test_check_output_exception is expected to fail now. Please remove that test.

Is it correct that the only difference between old code and this improvement is that the error is printed to log here:

error = "subprocess.check_output could not execute command ' " + str(args) + " '"
logger.error(error)

However this doesn't answer the original purpose of the issue in #703 :

There should ideally be a way for the user to know what the exact nature of the error is,

Please add code and tests that actually do that.

kirit93 · 2017-05-19T18:33:57Z

@tmylk, could you check out my latest push and let me know if that works for us?

kirit93 · 2017-05-29T09:41:55Z

@menshikh-iv, could you let me know if this PR is okay to merge?

menshikh-iv · 2017-05-29T17:03:49Z

-    Python 2.6.2
-    Added extra KeyboardInterrupt handling
+def check_output(args, flag=True):
+    r"""


Not required r

menshikh-iv · 2017-05-29T17:08:10Z

-    >>> check_output(args=['/usr/bin/python', '--version'])
-    Python 2.6.2
-    Added extra KeyboardInterrupt handling
+def check_output(args, flag=True):


A new interface breaks down many wrappers from gensim. First, please merge develop to your branch (it needs to correct test run).

Second, run tests locally with all wrappers like this
FT_HOME=/home/ivan/release/test/fastText WR_HOME=/home/ivan/release/test/wordrank VOWPAL_WABBIT_PATH=/home/ivan/release/test/vowpal_wabbit/vowpalwabbit/vw DTM_PATH=/home/ivan/release/test/dtm/dtm/main MALLET_HOME=/home/ivan/release/test/Mallet python setup.py test

Another variant is PR#1368 docker image. You can use it for full test run

My branch is upto date with develop. I ran python setup.py test with the appropriate env variable. Two tests failed but these are tests unrelated to what I am working on.

What should I do now?

@kirit93 hm, strange, I'll run all the tests and write the results here.

@menshikh-iv, It seems like the travis build succeeded after develop was merged into issue. Is the PR good to go?

menshikh-iv · 2017-06-03T14:18:01Z

I run all tests and get 39 errors, but they are all similar to each other:

First type

======================================================================
ERROR: testEnsemble (gensim.test.test_wordrank_wrapper.TestWordrank)
Test ensemble of two embeddings
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/ivan/release/test/gensim/gensim/test/test_wordrank_wrapper.py", line 37, in setUp
    self.test_model = wordrank.Wordrank.train(self.wr_path, self.corpus_file, self.out_name, iter=6, dump_period=5, period=5)
  File "/home/ivan/release/test/gensim/gensim/models/wrappers/wordrank.py", line 87, in train
    os.makedirs(meta_dir)
  File "/home/ivan/.virtualenvs/clean2/lib/python2.7/os.py", line 157, in makedirs
    mkdir(name, mode)
OSError: [Errno 17] File exists: '/home/ivan/release/test/wordrank/testmodel/meta'

Second type

======================================================================
ERROR: testLargeMmap (gensim.test.test_ldamallet_wrapper.TestLdaMallet)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/ivan/release/test/gensim/gensim/test/test_ldamallet_wrapper.py", line 61, in setUp
    self.model = ldamallet.LdaMallet(self.mallet_path, corpus, id2word=dictionary, num_topics=2, iterations=1)
  File "/home/ivan/release/test/gensim/gensim/models/wrappers/ldamallet.py", line 100, in __init__
    self.train(corpus)
  File "/home/ivan/release/test/gensim/gensim/models/wrappers/ldamallet.py", line 158, in train
    self.convert_input(corpus, infer=False)
  File "/home/ivan/release/test/gensim/gensim/models/wrappers/ldamallet.py", line 155, in convert_input
    check_output(args=cmd, shell=True)
TypeError: check_output() got an unexpected keyword argument 'shell'

Third type

======================================================================
ERROR: testCalledProcessError (gensim.test.test_dtm.TestDtmModel)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/ivan/release/test/gensim/gensim/test/test_dtm.py", line 70, in testCalledProcessError
    rng_seed=1)
  File "/home/ivan/release/test/gensim/gensim/models/wrappers/dtmmodel.py", line 129, in __init__
    self.train(corpus, time_slices, mode, model)
  File "/home/ivan/release/test/gensim/gensim/models/wrappers/dtmmodel.py", line 202, in train
    check_output(args=cmd, stderr=PIPE)
TypeError: check_output() got an unexpected keyword argument 'stderr'

For this reason, I can't merge your PR.

kirit93 · 2017-06-04T05:09:22Z

@menshikh-iv for the second two error types I can fix it by changing the definition of my check_output function to be like this def check_output(args, flag=True, stderr=None).

This way the way the function is called will not have to be changed in the other files in the code.
Another option is that I could change the calls to check_output so that they fit in with the new implementation.

And the first type of error is not related to check_output right?

menshikh-iv · 2017-06-04T14:21:28Z

@kirit93 First type error is strange, I will investigate it later. Could you please do a few things:

Return old interface for check_output function
Re-raise exception (if subprocess call breaks down)
Logging (with debug level) full command string with args, that user pass to check_output
Check case with KeyboardInterrupt (it should be possible to stop this call with Ctrl + C)

After this fixes, I will run all tests again and write a result to this PR.

kirit93 · 2017-06-09T17:21:54Z

@menshikh-iv I made the changes you requested and pushed them. Travis built successfully. Please let me know if the PR is okay now.

menshikh-iv · 2017-06-13T10:19:12Z

Sorry for late response @kirit93,

Now, I get a new kind of error, look at command

======================================================================
ERROR: testLargeMmap (gensim.test.test_ldamallet_wrapper.TestLdaMallet)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/ivan/release/test/gensim/gensim/test/test_ldamallet_wrapper.py", line 61, in setUp
    self.model = ldamallet.LdaMallet(self.mallet_path, corpus, id2word=dictionary, num_topics=2, iterations=1)
  File "/home/ivan/release/test/gensim/gensim/models/wrappers/ldamallet.py", line 100, in __init__
    self.train(corpus)
  File "/home/ivan/release/test/gensim/gensim/models/wrappers/ldamallet.py", line 158, in train
    self.convert_input(corpus, infer=False)
  File "/home/ivan/release/test/gensim/gensim/models/wrappers/ldamallet.py", line 155, in convert_input
    check_output(args=cmd, shell=True)
  File "/home/ivan/release/test/gensim/gensim/utils.py", line 1171, in check_output
    res = subprocess.check_output(args, shell=True)
  File "/usr/lib/python2.7/subprocess.py", line 219, in check_output
    raise CalledProcessError(retcode, cmd, output=output)
CalledProcessError: Command '/ h o m e / i v a n / r e l e a s e / t e s t / M a l l e t / b i n / m a l l e t   i m p o r t - f i l e   - - p r e s e r v e - c a s e   - - k e e p - s e q u e n c e   - - r e m o v e - s t o p w o r d s   - - t o k e n - r e g e x   " \ S + "   - - i n p u t   / t m p / 1 5 e 4 d b _ c o r p u s . t x t   - - o u t p u t   / t m p / 1 5 e 4 d b _ c o r p u s . m a l l e t' returned non-zero exit status 126

kirit93 · 2017-06-13T15:46:15Z

@menshikh-iv, I reverted to the old implementation of check_output and I ran the tests again. I still get an error like the one you posted.

ERROR: testCnpmiVWModel (gensim.test.test_coherencemodel.TestCoherenceModel)
Perform sanity check to see if c_npmi coherence works with LDA VW gensim wrapper
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/kirit/OpenSource/gensim/gensim/test/test_coherencemodel.py", line 70, in setUp
    self.malletmodel = LdaMallet(mallet_path=self.mallet_path, corpus=corpus, id2word=dictionary, num_topics=2, iterations=0)
  File "/Users/kirit/OpenSource/gensim/gensim/models/wrappers/ldamallet.py", line 100, in __init__
    self.train(corpus)
  File "/Users/kirit/OpenSource/gensim/gensim/models/wrappers/ldamallet.py", line 158, in train
    self.convert_input(corpus, infer=False)
  File "/Users/kirit/OpenSource/gensim/gensim/models/wrappers/ldamallet.py", line 155, in convert_input
    check_output(args=cmd, shell=True)
  File "/Users/kirit/OpenSource/gensim/gensim/utils.py", line 1206, in check_output
    raise error
subprocess.CalledProcessError: Command '/home/ivan/release/test/Mallet/bin/mallet import-file --preserve-case --keep-sequence --remove-stopwords --token-regex "\S+" --input /var/folders/gq/gqtxhd8j2cl0v9chmcjpd3mr0000gn/T/9de601_corpus.txt --output /var/folders/gq/gqtxhd8j2cl0v9chmcjpd3mr0000gn/T/9de601_corpus.mallet' returned non-zero exit status 127.

Could the cause of the error be something unrelated to the check_output code in this PR?

piskvorky · 2017-06-14T00:35:00Z

@kirit93 unless your system user is /home/ivan, that error log looks extremely fishy.

kirit93 · 2017-06-14T01:12:46Z

@piskvorky, I ran the following command FT_HOME=/home/ivan/release/test/fastText WR_HOME=/home/ivan/release/test/wordrank VOWPAL_WABBIT_PATH=/home/ivan/release/test/vowpal_wabbit/vowpalwabbit/vw DTM_PATH=/home/ivan/release/test/dtm/dtm/main MALLET_HOME=/home/ivan/release/test/Mallet python setup.py test

piskvorky · 2017-06-14T01:14:36Z

@kirit93 Where did you get that command from, do those paths really exist?

kirit93 · 2017-06-14T01:16:48Z

No they don't exist on my system, but @menshikh-iv asked me to run that command to test whether the PR works or not.

menshikh-iv · 2017-06-14T06:58:27Z

@kirit93 It's a path in my filesystem.

For the full test run, you should:

Install all external dependencies for wrappers (Fasttext, WordRank, WovpalVabbit, DTM, MALLET).
Create a clean virtual environment.
Install all test dependencies pip install .[test] (in gensim folder).
Substitute the necessary paths to environment variables and run a command from your message with correct paths for your filesystem.

menshikh-iv · 2017-06-14T07:00:54Z

@kirit93 I just ran all the tests on the development branch and made sure that everything works with the implementation from origin/develop.

kirit93 · 2017-06-14T16:30:28Z

@menshikh-iv, any idea what could be the cause of this error?

menshikh-iv · 2017-06-26T09:00:36Z

In my opinion, it is now easier to recreate PR and slightly change the code from develop branch.
Before that, please learn to run all tests for wrappers (instruction from previous post)

piskvorky changed the title ~~Issue #703. Added function to replace gensim check_output with subprocess.check_output~~ Added function to replace gensim check_output with subprocess.check_output Mar 6, 2017

tmylk suggested changes Mar 8, 2017

View reviewed changes

piskvorky requested changes Mar 9, 2017

View reviewed changes

tmylk reviewed Mar 19, 2017

View reviewed changes

tmylk suggested changes Mar 19, 2017

View reviewed changes

kirit93 commented Mar 19, 2017

View reviewed changes

kirit93 added 4 commits March 24, 2017 00:23

Added test case to check if exception is raised from check_output and…

4652241

… replaced deprecated assertEquals with assertEqual

Imported __future__ print_function for python2.7 compatibility

c22c418

Logging error instead of printing

b379418

Corrected small error in check_output

b73e0f9

tmylk reviewed Mar 28, 2017

View reviewed changes

Reverting to previous error handling logic

dd5abb1

tmylk reviewed May 2, 2017

View reviewed changes

kirit93 added 7 commits May 10, 2017 21:58

Logging error message and added assertion for exception

f2d3b91

Minor documentation change

2c01e0f

Removed trailing whitespace

040576d

Resolved more whitespace issues

874ca71

More whitespace errors resolved

b9f1bd1

Conforms to pep8

8255994

Merge branch 'develop' into issue

11c9cbc

piskvorky assigned menshikh-iv May 29, 2017

menshikh-iv suggested changes May 29, 2017

View reviewed changes

kirit93 added 2 commits May 30, 2017 11:48

Removed r from docstring

85762b4

Merge branch 'issue' of https://github.com/kirit93/gensim into issue

31b7397

kirit93 closed this May 31, 2017

kirit93 reopened this May 31, 2017

kirit93 added 3 commits June 9, 2017 22:01

Changing check output interface to work with calls to old implementation

720dd27

Added flake8 fixes

0d4e8c9

Merge branch 'develop' into issue

9bef24f

menshikh-iv closed this Jun 26, 2017

tmylk mentioned this pull request Jul 23, 2017

proper check for filename before calling subprocess #1485

Open

Uh oh!

Conversation

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

Uh oh!

Uh oh!

tmylk commented Mar 5, 2017

Uh oh!

kirit93 commented Mar 6, 2017

Uh oh!

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

Uh oh!

Uh oh!

kirit93 commented Mar 6, 2017

Uh oh!

tmylk commented Mar 6, 2017

Uh oh!

kirit93 commented Mar 7, 2017

Uh oh!

tmylk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kirit93 Mar 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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

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!

kirit93 Mar 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

tmylk left a comment

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!

kirit93 Mar 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tmylk commented Mar 23, 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!

kirit93 commented Mar 4, 2017 •

edited

Loading

piskvorky commented Mar 6, 2017 •

edited

Loading

kirit93 Mar 9, 2017 •

edited

Loading

piskvorky Mar 9, 2017 •

edited

Loading

kirit93 Mar 9, 2017 •

edited

Loading

kirit93 Mar 19, 2017 •

edited

Loading

piskvorky Apr 9, 2017 •

edited

Loading

kirit93 Apr 9, 2017 •

edited

Loading

piskvorky Apr 9, 2017 •

edited

Loading

piskvorky Apr 9, 2017 •

edited

Loading

kirit93 May 8, 2017 •

edited

Loading