Some patches to (a) show the reports and (b) fix some little bugs

6 views
Skip to first unread message

Rory McCann

unread,
Apr 8, 2010, 8:54:27 AM4/8/10
to django-lean
Hi,

I have been investigating using django-lean for my webapp, and at first
I couldn't see where to get the reports. I eventually had to step
through the source code. Here are some patches to make it a bit more
obvious. I'm not sure these are the best way to do it. I'm curious as to
how others get at this data. Perhaps I missed something obvious?

I've forked it on bitbucket and put my changes here:
http://bitbucket.org/rorymcc/django-lean

I've added "/experiments/admin" url to show the reports. I couldn't see
these view functions being used anywhere, so I put them in
"/experiments/admin". It's not a great solution since there is now
"/admin/experiments" and "/experiments/admin", both doing different things.
http://bitbucket.org/rorymcc/django-lean/changeset/e9c5d17349b2/

This one is a bug, where the url(...) function was failing for some of
the admin urls. This is a dirty hack to make it pick up those urls.
However I think this lets anyone (i.e. non-authenticated users) look at
the experiments, so it's *ahem* less than ideal.
http://bitbucket.org/rorymcc/django-lean/changeset/43bcc4d10812/

Some little bugs I noticed:

The 'conversion report' page was referring to 'engagement'
http://bitbucket.org/rorymcc/django-lean/changeset/ad66a355715d/

Don't show 'no data available' for days that are in the future (since
one obviously isn't going to have any data for them yet)
http://bitbucket.org/rorymcc/django-lean/changeset/9f1e14c03256/


Hope this helps

Rory McCann

signature.asc

Erik Wright

unread,
Apr 8, 2010, 10:38:18 AM4/8/10
to django-lean
Hi Rory,

Thanks for the patches. Here are some comments:

> I've added "/experiments/admin" url to show the reports. I couldn't see
> these view functions being used anywhere, so I put them in
> "/experiments/admin". It's not a great solution since there is now
> "/admin/experiments" and "/experiments/admin", both doing different things.http://bitbucket.org/rorymcc/django-lean/changeset/e9c5d17349b2/

We add this include within our main urls.py at 'admin/experiment-
reports/'. I think that most projects will want to choose where this
goes. So rather than hard-coding this in the main experiment urls.py I
would document this in the Wiki. For example, in our site:

ADMIN_PATH = settings.ADMIN_PATH.strip('/')

urlpatterns += patterns('',
...
url(r'^%s/experiment-reports/' % ADMIN_PATH,
include('experiments.admin_urls')),
...
)

> This one is a bug, where the url(...) function was failing for some of
> the admin urls. This is a dirty hack to make it pick up those urls.
> However I think this lets anyone (i.e. non-authenticated users) look at
> the experiments, so it's *ahem* less than ideal.http://bitbucket.org/rorymcc/django-lean/changeset/43bcc4d10812/

This is a bug that was introduced here:
http://bitbucket.org/rorymcc/django-lean/changeset/65fecaee82b1/

The proper fix is to give a name to the URL mapping and to pass the
name to 'reverse' instead of the view function.

> The 'conversion report' page was referring to 'engagement'http://bitbucket.org/rorymcc/django-lean/changeset/ad66a355715d/

Great catch. Thanks.

> Don't show 'no data available' for days that are in the future (since
> one obviously isn't going to have any data for them yet)http://bitbucket.org/rorymcc/django-lean/changeset/9f1e14c03256/

Actually, the system is not really designed to allow you to schedule
an experiment to end in the future. At the moment the end date is
automatically set when you transition an experiment to PROMOTED or
DISABLED from ENABLED.

There's nothing wrong with this patch, but I would be careful if you
are trying to schedule experiments to end in the future. I don't think
it will work (at least, not as expected).

In summary, what I would suggest is that you (1) revert the first and
second changesets, (2) document the addition of the admin URLs in the
wiki and (3) add a changeset that gives names to the admin URLs and
uses them in reverse. Then I would be delighted to merge your changes
back into the main tree.

Thanks for your interest in Django-Lean!

Erik

Rory McCann

unread,
Apr 8, 2010, 2:18:11 PM4/8/10
to djang...@googlegroups.com
Hi Erik, et al.,

On 08/04/10 15:38, Erik Wright wrote:
> We add this include within our main urls.py at 'admin/experiment-
> reports/'. I think that most projects will want to choose where this
> goes. So rather than hard-coding this in the main experiment urls.py I
> would document this in the Wiki. For example, in our site:
>
> ADMIN_PATH = settings.ADMIN_PATH.strip('/')
>
> urlpatterns += patterns('',
> ...
> url(r'^%s/experiment-reports/' % ADMIN_PATH,
> include('experiments.admin_urls')),
> ...
> )

Ah good point. I've updated the wiki to show this (wiki changeset
http://bitbucket.org/akoha/django-lean/wiki/changeset/3ba7e3a0b276/)

I've removed my 'fix' here
http://bitbucket.org/rorymcc/django-lean/changeset/c210ed73b436/

> The proper fix is to give a name to the URL mapping and to pass the
> name to 'reverse' instead of the view function.

OK, I've done that, the change is here (I think, I'm still getting used
to mercurial)
http://bitbucket.org/rorymcc/django-lean/changeset/be22b60eb76c/

>> Don't show 'no data available' for days that are in the future (since
>> one obviously isn't going to have any data for them yet)http://bitbucket.org/rorymcc/django-lean/changeset/9f1e14c03256/
>
> Actually, the system is not really designed to allow you to schedule
> an experiment to end in the future. At the moment the end date is
> automatically set when you transition an experiment to PROMOTED or
> DISABLED from ENABLED.
>
> There's nothing wrong with this patch, but I would be careful if you
> are trying to schedule experiments to end in the future. I don't think
> it will work (at least, not as expected).
>

Ah I see. I was just playing around with it and thought you had to set
an 'end date', so I set it about a month in the future. This then causes
lots of "No data available" rows.

So this patch will make the reports nicer for people who have set the
end date.

--------

I've added another change here:
http://bitbucket.org/rorymcc/django-lean/changeset/7ff5ce0b2fdb/

This will automatically create goals when you try to record that they
happen.

This isn't a bug fix per se. I like just being able to sprinkle "Goal
here", "Goal there" in my code without having to pre-create them.
However the 'do things behind my back' style might not be everyone's
taste, so it's up to you.


Rory


signature.asc

Erik Wright

unread,
Apr 8, 2010, 5:18:52 PM4/8/10
to django-lean
Awesome!

I imported your patches into the main trunk with a few small changes:

(1) I fixed the bug you reported some time ago about a test that fails
when you add Django-Lean to a fresh project without defining a
LEAN_ENGAGEMENT_CALCULATOR.
(2) I defined 'LEAN_AUTOCREATE_GOAL_TYPES' as a setting. Default is
False, so everyone else gets the previous behaviour and you get what
you want (by setting it to True in your settings.py).
(3) I wrote a test for (2).

Thanks for your contributions!

-Erik

On Apr 8, 2:18 pm, Rory McCann <r...@technomancy.org> wrote:
> Hi Erik, et al.,
>
> On 08/04/10 15:38, Erik Wright wrote:
>
> > We add this include within our main urls.py at 'admin/experiment-
> > reports/'. I think that most projects will want to choose where this
> > goes. So rather than hard-coding this in the main experiment urls.py I
> > would document this in the Wiki. For example, in our site:
>
> > ADMIN_PATH = settings.ADMIN_PATH.strip('/')
>
> > urlpatterns += patterns('',
> > ...
> >     url(r'^%s/experiment-reports/' % ADMIN_PATH,
> >         include('experiments.admin_urls')),
> > ...
> > )
>
> Ah good point. I've updated the wiki to show this (wiki changesethttp://bitbucket.org/akoha/django-lean/wiki/changeset/3ba7e3a0b276/)
>
> I've removed my 'fix' herehttp://bitbucket.org/rorymcc/django-lean/changeset/c210ed73b436/
>
> > The proper fix is to give a name to the URL mapping and to pass the
> > name to 'reverse' instead of the view function.
>
> OK, I've done that, the change is here (I think, I'm still getting used
> to mercurial)http://bitbucket.org/rorymcc/django-lean/changeset/be22b60eb76c/
>
> >> Don't show 'no data available' for days that are in the future (since
> >> one obviously isn't going to have any data for them yet)http://bitbucket.org/rorymcc/django-lean/changeset/9f1e14c03256/
>
> > Actually, the system is not really designed to allow you to schedule
> > an experiment to end in the future. At the moment the end date is
> > automatically set when you transition an experiment to PROMOTED or
> > DISABLED from ENABLED.
>
> > There's nothing wrong with this patch, but I would be careful if you
> > are trying to schedule experiments to end in the future. I don't think
> > it will work (at least, not as expected).
>
> Ah I see. I was just playing around with it and thought you had to set
> an 'end date', so I set it about a month in the future. This then causes
> lots of "No data available" rows.
>
> So this patch will make the reports nicer for people who have set the
> end date.
>
> --------
>
> I've added another change here:http://bitbucket.org/rorymcc/django-lean/changeset/7ff5ce0b2fdb/
>
> This will automatically create goals when you try to record that they
> happen.
>
> This isn't a bug fix per se. I like just being able to sprinkle "Goal
> here", "Goal there" in my code without having to pre-create them.
> However the 'do things behind my back' style might not be everyone's
> taste, so it's up to you.
>
> Rory
>
>  signature.asc
> < 1KViewDownload


--
Subscription settings: http://groups.google.com/group/django-lean/subscribe?hl=en

Rory McCann

unread,
Apr 25, 2010, 2:08:49 PM4/25/10
to djang...@googlegroups.com
Inspired by the 'auto-create goals' I have made similar changes for
experiments. I like just adding experiments to my templates, and these
patches enable that. Like 'autocreating goals', you have to explicitly
allow this.

You need these changes:
http://bitbucket.org/rorymcc/django-lean/changeset/8d416de9cf73 and
http://bitbucket.org/rorymcc/django-lean/changeset/1d5d5ddd0ca4

Another change I've made to my install is here
http://bitbucket.org/rorymcc/django-lean/changeset/4e1e425d47b4 This
will automatically create anonymous users even if it has not detected
any tests that user is enrolled in. I needed this to make things work.
Without it I found that users were not getting the visitor_id in the
session, and hence, I could not auto create experiments. It could be
that I was doing things in the wrong order or something.

Another change was to make it create experiment reports for today.
(http://bitbucket.org/rorymcc/django-lean/changeset/b7eeaeca9660 and
http://bitbucket.org/rorymcc/django-lean/changeset/1ac3a22b3e84 ). I'm
an analytics junkie and would like to see things as they happen. Once
again, you need to explicitly set LEAN_CALC_REPORTS_FOR_TODAY to enable
it. However I recently saw an article saying that you shouldn't look at
A/B test before they are done, so I'm not sure if this is such a clever
idea.

signature.asc

Erik Wright

unread,
Apr 30, 2010, 11:32:02 AM4/30/10
to django-lean
Hi Rory,

Thanks for the contributions. Sorry for the delay in moderating your
message - I was on holiday. The moderation is essential to prevent
spam but for some reason the 'always allow' feature that should enable
me to permanently grant permission to previous posters does not seem
to work.

Here is some feedback:

1) Previously, in DEBUG mode, by default, a missing test definition
caused an exception to be raised (as opposed to a simple log message
in production mode). Can you preserve this distinction?
2) Can you add a test for experiment auto-creation (both on and off)?
See test_models.py / testMissingGoalType for an example.
3) I think there is a bug in experiments/reports.py. If you run the
reports at noon today, and you enable mid-day report generation, a
report will be generated for today. If you run it again at 6PM, the
report will be updated. But if you next run it at 2AM the next day, it
appears (from a glance at the source) that the 6PM report will not be
updated. I see two possible fixes: (a) always re-run the last
generated report, whatever day it is or (b) mark mid-day reports as
provisional (perhaps by recording the generation time), and always re-
run all provisional reports.
4) Can you add a regression test for the bug I just described, along
with the fix)
5) As far as I'm concerned, we do not need an option for 'generate
today's reports'. It's OK to make this be the default for everyone.
But there needs to be a unit test for it.
6) I don't really understand why this change (http://bitbucket.org/
rorymcc/django-lean/changeset/4e1e425d47b4) was required. I looked at
the source to models.py / Experiment.__test_group and it seems like it
would auto-create the experiment before any inspection of the
anonymous user. Can you give me more details on the behaviour you saw?
Perhaps try to revert the change and verify that it is still broken?

Thanks,

Erik

On Apr 25, 2:08 pm, Rory McCann <r...@technomancy.org> wrote:
> Inspired by the 'auto-create goals' I have made similar changes for
> experiments. I like just adding experiments to my templates, and these
> patches enable that. Like 'autocreating goals', you have to explicitly
> allow this.
>
> You need these changes:http://bitbucket.org/rorymcc/django-lean/changeset/8d416de9cf73andhttp://bitbucket.org/rorymcc/django-lean/changeset/1d5d5ddd0ca4
>
> Another change I've made to my install is herehttp://bitbucket.org/rorymcc/django-lean/changeset/4e1e425d47b4This
> will automatically create anonymous users even if it has not detected
> any tests that user is enrolled in. I needed this to make things work.
> Without it I found that users were not getting the visitor_id in the
> session, and hence, I could not auto create experiments. It could be
> that I was doing things in the wrong order or something.
>
> Another change was to make it create experiment reports for today.
> (http://bitbucket.org/rorymcc/django-lean/changeset/b7eeaeca9660andhttp://bitbucket.org/rorymcc/django-lean/changeset/1ac3a22b3e84). I'm
>  signature.asc
> < 1KViewDownload
Reply all
Reply to author
Forward
0 new messages