Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overhaul experiment result stats page #3574

Merged
merged 19 commits into from Jun 30, 2017
Merged

Conversation

@nilbus
Copy link

@nilbus nilbus commented Jun 29, 2017

As promised in exercism/discussions#123

Page: /stats

Changes

  • Aggregate values for each time period rather than each day. This helps reasoning about differences between periods, which is difficult in the earlier format.
  • Explain what the experiment results mean.
  • Zoom to exclude major outliers by default. Otherwise, the significant region is too small to read.

Graph Before

Before

Graph After

After

Explanation Preview

explanations

nilbus added 19 commits Apr 27, 2017
Looking at review lengths day-by-day is not useful.
This aggregates all days for the same period.
Looking at review lengths day-by-day is not useful.
This was fun to watch during the experiment but is not useful afterward.
The variations by day aren't a meaningful variable in the experiment.
ParticipationStats already knows the dates that we are passing in.
Why require they be passed?
Moving away from date-based charts.
Because of the nature of the counting query, users who submitted 0
comments during a period were not represented in the statistics.
This was apparent because the number of comments per user in each period
was never reported to be less than 1.

This is difficult to accomplish in the original counting query, so
instead this patch fills 0s for each participating user (who commented
at least once during the experiment period) who isn't already
represented by a comment count.
Trying to fit all data into the graph makes none of the meaningful data
visible. You can still zoom out to see the full dataset.
Conflicts:
	app/routes/stats.rb

The side stats menu is now selecting tracks with some number of
implementations rather than problems.
@nilbus
Copy link
Author

@nilbus nilbus commented Jun 29, 2017

Leaving the participation_stats feature flag in for now, until after this new implementation is shown to perform well in production.

@nilbus nilbus changed the title Overhaul experiment result stats page, as promised in exercism/discussions#123. Overhaul experiment result stats page Jun 29, 2017
@kytrinyx
Copy link
Member

@kytrinyx kytrinyx commented Jun 30, 2017

This is pretty sweet. Thanks for taking the time to make this more understandable!

@kytrinyx kytrinyx merged commit 0a0e89a into exercism:master Jun 30, 2017
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.07%) to 86.498%
Details
@nilbus nilbus deleted the nilbus:participation-stats branch Jul 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants