Code generate TextLoader API and enhance it with convenience API. #142

codemzs · 2018-05-14T08:42:57Z

*Code generate support for IDataLoader
*Make TextLoader API code generated so that it's at functional parity with the text loader in the ML.Net infrastructure.
*Move TextLoader API under Microsoft.ML.Data namespace
*Add convenience TextLoader API.
*Add error checking for invalid loader arguments such as ordinal, column names.
*Update baselines.
*Update samples with new loader API and backward compatibility with old loader API.

ADDRESS COMMENTS FROM PR# 38
fixes #15

…er API.

…to loader

…test data file instead of train file.

…to loader

…to loader # Conflicts: # src/Microsoft.ML/TextLoader.cs

TomFinley · 2018-05-14T15:37:11Z

src/Microsoft.ML/Runtime/EntryPoints/ImportTextData.cs

+            [Argument(ArgumentType.Required, ShortName = "data", HelpText = "Location of the input file", SortOrder = 1)]
+            public IFileHandle InputFile;
+
+            [Argument(ArgumentType.Required, ShortName = "args", HelpText = "Arguments", SortOrder = 1)]


1 [](start = 101, length = 1)

SortOrder is the same here. #Closed

TomFinley · 2018-05-14T15:38:12Z

src/Microsoft.ML.Core/Data/DataKind.cs

            else if (type == typeof(Double))
                kind = DataKind.R8;
-            else if (type == typeof(DvText))
+            else if (type == typeof(DvText) || type == typeof(string))


Hi Zeeshan, this mapping here is inappropriate. Actually the below bool below is wrong as well. This is not a many-to-one utility mapping of "close enough." It means that if something has that data kind, then the underlying .NET representation type will be that, and vice versa.

Remove both. You'll unfortunately have to figure out what code is relying on the bool mistake below and fix that too, to use something more appropriate... #Closed

There's another utility function I mentioned in the other code review, GetDataType, in the ModuleArgs.cs that has this more "informal" mapping. But assuredly the core definitions of IDataView and its supporting type structures are not the place to have this sort of stuff. :D

In reply to: 188000280 [](ancestors = 188000280)

TomFinley · 2018-05-14T15:42:29Z

src/Microsoft.ML.Data/DataLoadSave/Text/TextLoader.cs

+            [Argument(ArgumentType.AtMostOnce, Visibility = ArgumentAttribute.VisibilityType.CmdLineOnly, HelpText = "Source column separator. Options: tab, space, comma, single character", ShortName = "sep")]
            public string Separator = "tab";

+            [Argument(ArgumentType.AtMostOnce, Visibility = ArgumentAttribute.VisibilityType.EntryPointsOnly, HelpText = "Source column separator. Option: single character ONLY", ShortName = "del")]


Option: single character ONLY [](start = 147, length = 29)

I think the fact that it's a single character is implied by the fact that the argument is of type char. We don't usually clarify that non-array arguments can only have a single value specified, since that information is part of the type anyway. #Closed

Though it would be nice if this argument was as capable as Separator, which can support multiple delimiters.

In reply to: 188001984 [](ancestors = 188001984)

When you shift to multiple delimiters, take the same care as is taken with Separator to ensure uniqueness, etc.

In reply to: 188006909 [](ancestors = 188006909,188001984)

TomFinley · 2018-05-14T15:46:14Z

src/Microsoft.ML.Data/DataLoadSave/Text/TextLoader.cs


            _host.CheckNonEmpty(args.Separator, nameof(args.Separator), "Must specify a separator");
-            string sep = args.Separator.ToLowerInvariant();
+            _host.CheckNonEmpty(args.Delimiter.ToString(), nameof(args.Delimiter), "Must specify a delimeter");


args.Delimiter.ToString(), [](start = 32, length = 26)

What is the purpose of this check? Under what circumstances do we suppose that a character, when represented as a string, will be empty? #Closed

TomFinley · 2018-05-14T15:52:58Z

src/Microsoft.ML.Data/DataLoadSave/Text/TextLoader.cs

-            if (sep == ",")
-                _separators = new char[] { ',' };
+            if (args.Delimiter != '\t')
+                _separators = new char[] { args.Delimiter };


Maybe comment on the code how this relies on the fact that the default arg.Separator is "tab", since to a reader this will look odd. That is, it may look like a mistake, even though it isn't one. #Closed

TomFinley · 2018-05-14T15:55:03Z

src/Microsoft.ML.PipelineInference/Microsoft.ML.PipelineInference.csproj


+  <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'">
+    <NoWarn>;1591;0618</NoWarn>
+  </PropertyGroup>


Could we clarify this? What warnings are these, and what is the relationship to your change? #Closed

I had added 0618 but now I have removed it and applied to code sections.

In reply to: 188006377 [](ancestors = 188006377)

TomFinley · 2018-05-14T16:04:23Z

src/Microsoft.ML.Data/DataLoadSave/Text/TextLoader.cs

+            [Argument(ArgumentType.AtMostOnce, Visibility = ArgumentAttribute.VisibilityType.CmdLineOnly, HelpText = "Source column separator. Options: tab, space, comma, single character", ShortName = "sep")]
            public string Separator = "tab";

+            [Argument(ArgumentType.AtMostOnce, Visibility = ArgumentAttribute.VisibilityType.EntryPointsOnly, HelpText = "Source column separator. Option: single character ONLY", ShortName = "del")]


Argument [](start = 13, length = 8)

This is a little unfortunate... on the one hand we have Separator. On the other hand we have Delimiter. I know you're doing this to avoid a name collision, but it would be nice if, from the user's perspective, the same concept had the same name in API usage and command line usage.

I think the Name attribute on Argument exists for this purpose. You can name the field whatever (maybe Delimiter could instead be SeparatorChar, or SeparatorChars if you fix it to be char[] instead). Then you can use Name = nameof(Separator) to make sure it has the same name in command line usage. The shortname I hope can I think be consistent between the two as well. #Closed

TomFinley · 2018-05-14T16:09:35Z

test/Microsoft.ML.Tests/TextLoaderTests.cs

-            Assert.NotNull(new TextLoader<Input>("fakeFile.txt", useHeader: false, separator: "tab", supportSparse: false));
-            Assert.NotNull(new TextLoader<Input>("fakeFile.txt", useHeader: false, separator: "tab", allowQuotedStrings: false));
+            Assert.NotNull(new TextLoader<Input>("fakeFile.txt", delimeter: ','));
+            Assert.NotNull(new TextLoader<Input>("fakeFile.txt", useHeader: false, delimeter: ','));


',' [](start = 94, length = 3)

I'm not sure it matters given the nature of this test, but was there any particular reason to shift from tab to ,?

Given how you've made \t an exceptional case in the constructor, might want to do both. #Closed

This was on an accident. Reverted back.

In reply to: 188011098 [](ancestors = 188011098)

TomFinley · 2018-05-14T16:12:46Z

test/Microsoft.ML.Predictor.Tests/Microsoft.ML.Predictor.Tests.csproj

  <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'">
    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
    <PlatformTarget>AnyCPU</PlatformTarget>
+    <NoWarn>1701;1702;1705;0618</NoWarn>


Perhaps if the warning really is inappropriate in one very specific case, we could mark the very specific case rather than suppressing it. By doing this, forevermore, we will never, under any circumstances, be aware when these warning is generated. And if I look up some of these, some of these are good warnings, things we will probably want to be aware of. #Closed

TomFinley · 2018-05-14T16:16:07Z

test/Microsoft.ML.TestFramework/ModelHelper.cs

+                        new TextLoaderColumn()
+                        {
+                            Name = "Id",
+                            Source = new [] { new TextLoaderRange() { Min = 0, Max = 0} },


[](start = 40, length = 1)

Maybe VS could format the file to clean up thse whitespace issues, I think default VS formatting is new[] not new []. #Closed

I tried ctrl-k-d but still there is space.

In reply to: 188013046 [](ancestors = 188013046)

Ah. How strange. Oh well.

In reply to: 188121093 [](ancestors = 188121093,188013046)

TomFinley · 2018-05-14T16:22:59Z

src/Microsoft.ML/CSharpApi.cs

+            /// </summary>
+            public KeyRange KeyRange { get; set; }
+
+        }


An observation:

These classes are generated from the entry-point, which is somewhat unfortunate since we don't have the ability to set some common conveniences. Like say, we have TextLoaderRange[] Source, but there are two common cases: the case where only a single range is specified for the vector valued column case, and the case where only a single value is specified. If we could somehow add to this class, we could add some conveniences to allow this to be set.

I wonder: if we changed the CSharpCodeGenerator.cs code to make these partial classes, then we could keep this auto-generated file. But then there could be another file in the same assembly containing hand written enhancements to these classes to make them more convenient. I suppose it all depends on whether we view these classes as being something the user will often directly interact with (in which case, a convenience may be appropriate), or whether our view is that these entry-point generated APIs will generally be wrapped, as is currently the case in this Microsoft.ML./Data/TextLoader.cs file. (in which case convenience at this level are irrelevant).

If we did have partial classes, then we could add some facilities to TextLoaderColumn maybe, to allow (1) single column setting, or (2) single range setting. Or add them as constructors to TextLoaderRange itself, either way. #Closed

I too wanted to do this as I converted test cases to use the code generated API and realized it was inconvenient to specify an ordinal twice when it could be done by specifying once.

In reply to: 188015114 [](ancestors = 188015114)

TomFinley · 2018-05-14T16:42:47Z

src/Microsoft.ML/Data/TextLoader.cs

+                var col = Runtime.Data.TextLoader.Column.Parse(
+                    $"{name}:" +
+                    $"{dk.ToString()}:" +
+                    $"{mappingAttr.Ordinal}"


This is unnecessary. We only needed to parse out Ordinal. We could use TextLoader.Range(Ordinal) for that, and take the name and kind as is.

Alternately, we could exploit the fact that we now have a range type defined, and use that instead of making our users learn the mini-language. #Closed

Still a big improvement over the old system of course.

In reply to: 188021126 [](ancestors = 188021126)

TomFinley · 2018-05-14T16:45:21Z

src/Microsoft.ML/CSharpApi.cs

+        /// Import a dataset from a text file
+        /// </summary>
+        [Obsolete("Use TextLoader instead.")]
+        public sealed partial class CustomTextLoader


Nice #ByDesign

TomFinley · 2018-05-14T16:48:15Z

src/Microsoft.ML/Runtime/EntryPoints/ImportTextData.cs


-        [TlcModule.EntryPoint(Name = "Data.TextLoader", Desc = "Import a dataset from a text file")]
+#pragma warning disable 0618
+        [Obsolete("Use TextLoader instead.", false)]


, false [](start = 43, length = 7)

This is the default for the overload without it specified anyway, so it can be omitted. #Closed

TomFinley · 2018-05-14T16:48:54Z

src/Microsoft.ML/CSharpApi.cs

        /// Import a dataset from a text file
        /// </summary>
-        public sealed partial class TextLoader
+        public partial class TextLoader : Microsoft.ML.ILearningPipelineLoader


public partial [](start = 8, length = 14)

So why not sealed? #Closed

Ah I see. Because of that other issue. Reseal, maybe make another part of the partial class to introduce a Create<TInput> convenience method that returns a TextLoader.

In reply to: 188022924 [](ancestors = 188022924)

TomFinley · 2018-05-14T17:21:37Z

src/Microsoft.ML/Data/TextLoader.cs

+
+namespace Microsoft.ML.Data
+{
+    public sealed class TextLoader<TInput> : TextLoader


TextLoader [](start = 45, length = 10)

What is the purpose of making this descend from the other?

AFAIK this thing has to be a ILearningPipelineLoader, and is intended to be a convenience wrapper over the original TextLoader. #Closed

Having read this, it looks like this "subclass"'s only job is to take the type and a few constructor arguments, and that's the beginning and end of what it is used for.

This is a fairly odd use of inheritance, one without parallel in our codebase. Further it violates the general practice of non-sealed/abstract classes. This ought to be structured differently, either as a static generic utility method on top of TextLoader, but to declare a new class just for that convenience is odd. If you really want it to be a separate class (which, really, it shouldn't be), then it'll have to be a wrapper as the old loader was structured to be.

In reply to: 188032656 [](ancestors = 188032656)

It's also misleading. Let's imagine that I have a TextLoader<T>. Is T used anywhere? Nope. Is T even slightly relevant? Not really -- by defining this to be an inheritance relationship, you are retaining the mutable properties that usual usage of TextLoader depends on. Which means, a user is free to change/ignore any information garnered from T. :) I know what a List<T> is. I know what an IEnumerable<T> is. I, even if I'm totally familiar with this class, have absolutely no idea whatsoever what a TextLoader<T> is.

We're using inheritance, but as soon as we rely on any of the schema setting functionality of that base class, the fact that this type is generic on <T> is no longer useful information. That's I'd say another sign something's wrong.

In reply to: 188038035 [](ancestors = 188038035,188032656)

TomFinley · 2018-05-14T17:56:57Z

src/Microsoft.ML.Core/EntryPoints/ModuleCatalog.cs

        public sealed class EntryPointInfo
        {
+            public readonly bool NoSeal;
            public readonly string Name;


Let's revert this change of NoSeal. (The other change, the ObsoleteAttribute change is good, so please don't revert the whole file.)

Firstly, there's the point as made elsewhere that inheritance wasn't the right way to solve that problem, but there's a larger point...

The EntryPointAttribute is a description of the component. You wanted that class to be unsealed specifically because you had an idea of how to structure the C# API specifically. Setting aside the merits of that idea, or even supposing that it was a good idea, we should avoid putting things that are so extremely specific stuff in one environment (in this case, the code in CSharpApiGenerator.cs) in the entry-point definition meant to service everyone. Let's think about everything else in this info class... it's useful and necessary practically everywhere. This is not the case with NoSeal.
#Closed

TomFinley · 2018-05-14T18:01:29Z

src/Microsoft.ML/TextLoader.cs

-        private string _inputFilePath;
-        private string CustomSchema;
-        private Data.TextLoader ImportTextInput;
-


I'm not a fan of this old code, but it had the benefit that TInput meant something, since we had assurances (as it was wrapped as a private field) that the schema produced by this thing would be faithful to the type. (That's not a good reason to make a generic type, but it is at least somewhat understandable.) Now we're doing inheritance, it means nothing. If you want to generate a TextLoader, make a method to generate a text loader. #Closed

…to loader # Conflicts: # src/Microsoft.ML/LearningPipeline.cs

TomFinley · 2018-05-17T17:57:28Z

src/Microsoft.ML/Data/TextLoader.cs

+        public TextLoaderRange(int ordinal)
+        {
+
+            Contracts.Assert(ordinal >= 0);


Contracts.Assert(ordinal >= 0); [](start = 12, length = 31)

Not asserts! These are public facing methods. #Closed

More detail: Asserts are debug only constructs, and we only use them to indicate what we expect is true -- e.g., verification of invariants in code. These should be .CheckParam, etc.

In reply to: 189048133 [](ancestors = 189048133)

TomFinley · 2018-05-17T19:58:33Z

src/Microsoft.ML/Data/TextLoader.cs

+
+        /// <summary>
+        /// Convenience constructor for the scalar case.
+        /// Min and Max are set to the single value ordinal.


Min [](start = 12, length = 3)

<see, also ordinal should be in a <paramref. #Closed

@TomFinley These rules could be enforced as errors too. Though the problem currently is that many of the interfaces aren't commented. I'm not sure if going in stages could be a viable alternative.

A good take on commenting: dotnet/orleans#209 (comment). #Resolved

Thanks, Veikko.

In reply to: 189102883 [](ancestors = 189102883)

Thanks for adding the <paramref. Min and max still not in a <see

In reply to: 189082232 [](ancestors = 189082232)

TomFinley · 2018-05-17T20:02:24Z

src/Microsoft.ML/Data/TextLoader.cs

+        /// Convenience constructor for the scalar case.
+        /// Min and Max are set to the single value ordinal.
+        /// When a given column spans only a single column in the
+        /// dataset.


You're using two distinct meanings of the word column in one sentence. This is fairly confusing. #Closed

TomFinley · 2018-05-17T20:03:35Z

src/Microsoft.ML/Data/TextLoader.cs

+        /// Convenience constructor for the scalar case.
+        /// Min and Max are set to the single value ordinal.
+        /// When a given column spans only a single column in the
+        /// dataset.


This is not a sentence. It almost works if you were to append it to the first sentence, followed by a comma. #Closed

I concur, as a non-native speaker, I'd go to read the code (and be slightly annoyed). :) #Resolved

Still not a sentence. Perhaps reading this summary comment aloud to yourself would help?

In reply to: 189083569 [](ancestors = 189083569)

Oh OK... I see what went wrong here.

Append this third "sentence" to the first sentence followed by a comma.

By this I mean, I am not proposing the following transformation.

"I like to run. When the sun shines." => "I like to run. When the sun shines," because that's still not a sentence and its punctuation makes no sense.

I am proposing the following transformation.

"I like to run, when the sun shines."

In reply to: 189340150 [](ancestors = 189340150,189083569)

…to loader

TomFinley · 2018-05-18T18:11:24Z

src/Microsoft.ML/Data/TextLoader.cs

+        /// <summary>
+        /// Convenience constructor for the vector case.
+        /// When a given column in the schema spans contiguous columns in the dataset,
+        /// <see cref="ML.Runtime.Data.TextLoader.Range"/>


[](start = 12, length = 46)

So, why are we having code in the public API reference classes in the runtime namespaces? #Closed

TomFinley · 2018-05-18T18:12:56Z

src/Microsoft.ML/Data/TextLoader.cs

+        {
+
+            Contracts.CheckParam(min >= 0, nameof(min));
+            Contracts.CheckParam(max >= 0, nameof(max));


Max should be >= than min. You also do not have any message at all for the user for what actually happened.

The TextLoader class is arguably one of the most important components in this API, wouldn't we say? If a user misuses it, and we throw an exception, we really ought to be clear. #Closed

TomFinley · 2018-05-18T18:20:02Z

src/Microsoft.ML/Data/TextLoader.cs

+        /// When a given column in the schema spans contiguous columns in the dataset,
+        /// <see cref="ML.Runtime.Data.TextLoader.Range"/>
+        /// </summary>
+        public TextLoaderRange(int min, int max)


<param tags? Isn't it important to explain these? #Closed

TomFinley · 2018-05-21T17:01:46Z

src/Microsoft.ML/Data/TextLoader.cs

+        {
+
+            Contracts.CheckParam(min >= 0, nameof(min), "Cannot be negative number.");
+            Contracts.CheckParam(max >= min, nameof(max), $"Cannot be less than {nameof(min)}.");


$"Cannot be less than {nameof(min)}." [](start = 58, length = 37)

Note that this $ interpolation means this isn't a constant. Note however, that "Cannot be less than " + nameof(min) + "." would be a constant, and so we could use it in a Check error message. #Resolved

TomFinley · 2018-05-21T17:02:32Z

src/Microsoft.ML/Data/TextLoader.cs

+        public TextLoaderRange(int min, int max)
+        {
+
+            Contracts.CheckParam(min >= 0, nameof(min), "Cannot be negative number.");


Cannot be negative number. [](start = 57, length = 26)

Make consistent with the above "Cannot be a negative number." #Resolved

TomFinley

yaeldekel

…to loader # Conflicts: # src/Microsoft.ML.Core/Data/DataKind.cs

codemzs added 9 commits May 5, 2018 22:10

Code generated loader API and install it at the backend pipeline load…

3b88d2f

…er API.

Merge branch 'master' of https://github.com/dotnet/machinelearning in…

f46f36a

…to loader

Update baselines and add checks for invalid loader arguments.

c0ff881

Revert test metric changes and change test file for evaluation to be …

6d5f351

…test data file instead of train file.

cleanup.

51d5658

Merge branch 'master' of https://github.com/dotnet/machinelearning in…

24495fb

…to loader

Merge branch 'master' of https://github.com/dotnet/machinelearning in…

628cd1d

…to loader # Conflicts: # src/Microsoft.ML/TextLoader.cs

resolve conflits from upstream/master merge.

c5dd614

Feedback from PR#38

a61d244

codemzs requested review from eerhardt and TomFinley May 14, 2018 08:43

TomFinley reviewed May 14, 2018

View reviewed changes

TomFinley mentioned this pull request May 14, 2018

Code generate TextLoader API and make it backward compatible with existing TextLoader API. #38

Closed

shauheen added the API Issues pertaining the friendly API label May 17, 2018

codemzs added 3 commits May 16, 2018 23:03

Merge branch 'master' of https://github.com/dotnet/machinelearning in…

b5c4fcb

…to loader # Conflicts: # src/Microsoft.ML/LearningPipeline.cs

PR feedback.

bb1afd4

PR feedback.

596079f

TomFinley reviewed May 17, 2018

View reviewed changes

codemzs added 2 commits May 17, 2018 21:01

Merge branch 'master' of https://github.com/dotnet/machinelearning in…

b1d44b3

…to loader

PR feedback.

6233b76

TomFinley reviewed May 18, 2018

View reviewed changes

PR feedback.

a6805c9

TomFinley reviewed May 21, 2018

View reviewed changes

TomFinley approved these changes May 21, 2018

View reviewed changes

yaeldekel approved these changes May 21, 2018

View reviewed changes

codemzs added 2 commits May 21, 2018 19:11

Merge branch 'master' of https://github.com/dotnet/machinelearning in…

ca98fc5

…to loader # Conflicts: # src/Microsoft.ML.Core/Data/DataKind.cs

PR feedback.

78e89c3

codemzs merged commit 86f5ee6 into dotnet:master May 22, 2018

codemzs deleted the loader branch May 22, 2018 02:32

TomFinley mentioned this pull request Jun 8, 2018

Enabled '_' in the field/column name in input type. #327

Merged

ghost locked as resolved and limited conversation to collaborators Mar 30, 2022

Code generate TextLoader API and enhance it with convenience API. #142

Code generate TextLoader API and enhance it with convenience API. #142

Uh oh!

Conversation

codemzs commented May 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TomFinley May 14, 2018 • 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!

TomFinley May 14, 2018 • 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!

TomFinley May 14, 2018 • 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!

TomFinley May 14, 2018 • 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!

TomFinley May 14, 2018 • 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!

TomFinley May 14, 2018 • 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!

TomFinley May 14, 2018 • 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!

TomFinley May 14, 2018 • 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!

TomFinley May 14, 2018 • 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!

TomFinley May 14, 2018 • 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!

TomFinley May 14, 2018 • 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!

TomFinley May 14, 2018 • 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!

TomFinley May 14, 2018 • 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!

TomFinley May 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

codemzs commented May 14, 2018 •

edited

Loading

TomFinley May 14, 2018 •

edited

Loading

TomFinley May 14, 2018 •

edited

Loading

TomFinley May 14, 2018 •

edited

Loading

TomFinley May 14, 2018 •

edited

Loading

TomFinley May 14, 2018 •

edited

Loading

TomFinley May 14, 2018 •

edited

Loading

TomFinley May 14, 2018 •

edited

Loading

TomFinley May 14, 2018 •

edited

Loading

TomFinley May 14, 2018 •

edited

Loading

TomFinley May 14, 2018 •

edited

Loading

TomFinley May 14, 2018 •

edited

Loading

TomFinley May 14, 2018 •

edited

Loading

TomFinley May 14, 2018 •

edited

Loading

TomFinley May 14, 2018 •

edited

Loading

TomFinley May 14, 2018 •

edited

Loading

TomFinley May 14, 2018 •

edited

Loading

TomFinley May 14, 2018 •

edited

Loading

TomFinley May 14, 2018 •

edited

Loading

TomFinley May 14, 2018 •

edited

Loading

TomFinley May 14, 2018 •

edited

Loading

TomFinley May 17, 2018 •

edited

Loading

TomFinley May 17, 2018 •

edited

Loading

TomFinley May 17, 2018 •

edited

Loading

veikkoeeva May 17, 2018 •

edited by codemzs

Loading

TomFinley May 18, 2018 •

edited

Loading

TomFinley May 17, 2018 •

edited

Loading