Stats refactor, with blocks this time

lib/modesty/experiment/base.rb

@@ -6,7 +6,7 @@ def initialize(slug)
     end
 
     def inspect
-      "#<Modesty::Experiment:(#{self.slug.inspect})>"
+      "#<Modesty::Experiment[ #{self.slug.inspect} ]>"
     end
 
     ATTRIBUTES = [

lib/modesty/experiment/stats.rb

@@ -2,46 +2,94 @@ module Modesty
   class Experiment
 
     def stats
-      @stats ||= []
+      @stats ||= Hash.new do |hash, key|
+        raise Error, <<-msg.squish
+          Unrecognized stat #{key.inspect}
+        msg
+      end
     end
 
     def reports(*args)
-      self.stats.map { |s| s.report(*args) }
+      self.stats.values.map { |s| s.report(*args) }
     end
 
     class Builder
-      def distribution(metric, options={})
-        @exp.stats << DistributionStat.new(@exp, metric, options)
+      def distribution(name, options={}, &blk)
+        @exp.stats[name] = DistributionStat.new(@exp, name, options, &blk)
+      end
+
+      def conversion(name, options={}, &blk)
+        @exp.stats[name] = ConversionStat.new(@exp, name, options, &blk)
+      end
+    end
+
+    class ArgumentProxy
+      def initialize(obj, *args)
+        @obj = obj
+        @args = args
       end
 
-      def conversion(num, denom, options={})
-        @exp.stats << ConversionStat.new(@exp, num, denom, options)
+      def inspect
+        "#<ArgumentProxy[ #{@obj.inspect} ]>"
+      end
+
+      def method_missing(meth, *args)
+        data = @obj.send(meth, *(args + @args))
+        # [Jay] #TODO: Hack alert!
+        # this doesn't take into account Metric#all,
+        # which returns an Array for either a date range
+        # or a single day
+        data = data.sum if data.is_a?(Array)
+        data
       end
     end
 
     class Stat
+      def initialize(exp, name, options={}, &blk)
+        @exp = exp
+        @name = name
+        @get_data = blk || default_get_data(options[:on])
+      end
+
+      def title
+        @name.to_s.split(/_/).map(&:capitalize).join(' ')
+      end
+
       def report(*args)
+        sig = significance(*args)
+        sig = "not significant" if sig.nil?
         return <<-report
 
-          ===#{title}===
-          #{significance(*args).inspect}
+          === #{title} ===
+          #{analysis(*args).inspect}
+          Significance: #{sig}
         report
       end
 
       def significant?(tolerance=0.01)
         sig = self.significance
         !sig.nil? && sig <= tolerance
       end
-    end
 
-    class DistributionStat < Stat
-      def initialize(exp, metric_sym, options={})
-        @exp = exp
-        @metric_sym = metric_sym
+      private
+      def argument_proxy_hash(hsh, *args)
+        Hash[
+          hsh.map do |k, v|
+            [k, ArgumentProxy.new(v, *args)]
+          end
+        ]
       end
 
-      def title
-        "Distribution stats on #{@exp.slug.inspect} for #{@metric_sym.inspect}"
+      def data_for(alt, *args)
+        data = @get_data.call(argument_proxy_hash(@exp.metrics(alt), *args))
+      end
+    end
+
+    class DistributionStat < Stat
+      def default_get_data(on_param)
+        lambda do |metrics|
+          metrics[on_param].distribution
+        end
       end
 
       def inspect
@@ -51,7 +99,7 @@ def inspect
       def data(*args)
         Hash[
           @exp.alternatives.map do |a|
-            [a, @exp.metrics(a)[@metric_sym].distribution(*args)]
+            [a, data_for(a, *args)]
           end
         ]
       end
@@ -67,26 +115,28 @@ def significance(*args)
     end
 
     class ConversionStat < Stat
-      def initialize(exp, num, denom, options={})
-        @exp = exp
-        @num_sym = num
-        @denom_sym = denom
+      def default_get_data(on_param)
+        lambda do |metrics|
+          num_count = metrics[on_param[0]].count
+          denom_count = metrics[on_param[1]].count
+          [num_count, denom_count - num_count]
+        end
       end
 
-      def title
-        "Count of #{@num_sym} out of #{@denom_sym}"
+      def analysis(*args)
+        Hash[
+          @exp.alternatives.map do |a|
+            [a, data_for(a, *args)]
+          end
+        ]
       end
 
       def data(*args)
-        @exp.alternatives.map do |a|
-          num_count = @exp.metrics(a)[@num_sym].count(*args)
-          denom_count = @exp.metrics(a)[@denom_sym].count(*args)
-          [num_count, denom_count - num_count]
-        end
+        analysis.values
       end
 
       def inspect
-        "#<Modesty::Experiment::ConversionStat[ (on #{@exp.slug}) (of #{@num_sym.inspect})/(#{@denom_sym.inspect}) ]>"
+        "#<Modesty::Experiment::ConversionStat[ #{@name} ]>"
       end
 
       def significance(*args)

spec/significance_spec.rb

@@ -11,10 +11,14 @@
 
     @e = Modesty.new_experiment :baz do |e|
       e.metrics :foo, :bar
-      e.conversion :foo, :bar
-      e.distribution :foo
+
+      e.conversion :foo_conv, :on => [:foo, :bar]
+      e.distribution :foo_dist, :on => :foo
     end
 
+    @foo_dist = @e.stats[:foo_dist]
+    @foo_conv = @e.stats[:foo_conv]
+
     Modesty.identify! 1
   end
 
@@ -29,50 +33,114 @@
       Modesty.track! :foo, rand(100)
     end
 
-    @e.stats[1].should be_a Modesty::Experiment::DistributionStat
-    an = @e.stats[1].analysis
+    @foo_dist.should be_a Modesty::Experiment::DistributionStat
+    an = @foo_dist.analysis
     an.should be_a Hash
     an[:control][:mean].should be_close(50, 10)
     an[:experiment][:mean].should be_close(100, 20)
     an[:control][:size].should == 250
     an[:experiment][:size].should == 250
 
-    sig = @e.stats[1].significance
+    sig = @foo_dist.significance
     sig.should be_a Float
     sig.should be < 0.01
-    @e.stats[1].should be_significant
+    @foo_dist.should be_significant
   end
 
   it "handles insignificant distribution data" do
     @e.chooses :experiment
     250.times do
-      Modesty.track! :foo, rand(10)
+      Modesty.track! :foo, 1+rand(10)
     end
     @e.chooses :control
     250.times do
-      Modesty.track! :foo, rand(10)
+      Modesty.track! :foo, 1+rand(10)
     end
 
-    sig = @e.stats[1].significance
+    sig = @foo_dist.significance
     sig.should be_nil
-    @e.stats[1].should_not be_significant
+    @foo_dist.should_not be_significant
   end
 
   it "handles significant conversion data" do
     @e.chooses :experiment
     500.times do
-      Modesty.track! :foo, rand(5)
+      Modesty.track! :foo, 1+rand(5)
       Modesty.track! :bar, 100
     end
 
     @e.chooses :control
     500.times do
-      Modesty.track! :foo, rand(100)
+      Modesty.track! :foo, 1+rand(100)
       Modesty.track! :bar, 100
     end
 
-    @e.stats[0].should be_a Modesty::Experiment::ConversionStat
-    sig = @e.stats[0].significance
+    @foo_conv.should be_a Modesty::Experiment::ConversionStat
+    sig = @foo_conv.significance
     sig.should_not be_nil
   end
 end
+
+describe "Statistics with blocks" do
+  before :each do
+    Modesty.data.flushdb
+    Modesty.metrics.clear
+    Modesty.experiments.clear
+
+    Modesty.new_metric :foo
+    Modesty.new_metric :bar
+
+    @e = Modesty.new_experiment :baz do |e|
+      e.metrics :foo, :bar
+
+      e.distribution :special_dist do |metrics|
+        metrics[:foo].distribution + metrics[:bar].distribution
+      end
+
+      e.conversion :special_conv do |metrics|
+        [
+          metrics[:foo].unique(:users),
+          metrics[:foo].count
+        ]
+      end
+    end
+
+    (1..500).each do |i| 
+      Modesty.identify!(i)
+      Modesty.group :baz
+      Modesty.track! :foo, 1+rand(i)
+      Modesty.track! :bar, 1+rand(501-i)
+    end
+
+  end
+
+  it "uses the blocks for distribution" do
+    three_days = @e.stats[:special_dist].data(3.days.ago..Date.today)
+    three_days.should == {
+      :control => (
+        @e.metrics(:control)[:foo].distribution(3.days.ago..Date.today).sum +
+        @e.metrics(:control)[:bar].distribution(3.days.ago, Date.today).sum
+      ),
+      :experiment => (
+        @e.metrics(:experiment)[:foo].distribution(3.days.ago, :today).sum +
+        @e.metrics(:experiment)[:bar].distribution(3.days.ago..Date.today).sum
+      )
+    }
+
+    three_days.should == @e.stats[:special_dist].data(3.days.ago, :today)
+  end
+
+  it "uses the blocks for conversion" do
+    three_days = @e.stats[:special_conv].data(3.days.ago..Date.today)
+    three_days.should == [
+      [
+        @e.metrics(:control)[:foo].unique(:users, 3.days.ago..Date.today).sum,
+        @e.metrics(:control)[:foo].count(3.days.ago, :today).sum
+      ],
+      [
+        @e.metrics(:experiment)[:foo].unique(:users, 3.days.ago, :today).sum,
+        @e.metrics(:experiment)[:foo].count(3.days.ago, Date.today).sum
+      ]
+    ]
+  end
+end