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

Add 7-day moving averages to download graph. Plus UI tweaks. #552

Merged
merged 3 commits into from
Feb 21, 2017

Conversation

hellp
Copy link
Contributor

@hellp hellp commented Feb 10, 2017

The non-stacked scattered chart as a basis reflects the download numbers
correctly. The stacked area chart before was a bit midleading. Both
approaches are a bit "noisy", so I added a 7-day moving average line for
each version number. This gives a higher-level overview and also makes it
easy to see when new versions overtake older ones in usage.

The colors were chosen to be colorblindness-friendly as well as having
a hot-cold metaphor.

Example, the futures crate, before and after:

screenshot-20170210-01701a

screenshot-20170210-01430a

@carols10cents
Copy link
Member

Thanks, this looks awesome!! 🎁

Since you sent in this PR, we've upgraded ember, which seems to have gotten rid of window._, so I pushed a commit to your branch importing lodash, which worked for me locally. I also merged in upstream and I'm going to wait to make sure CI is ok before I merge this in :)

@carols10cents
Copy link
Member

welp that didn't work, investigating....

@carols10cents
Copy link
Member

I think this is hitting ember-cli/ember-cli-qunit#162 now...

@hellp
Copy link
Contributor Author

hellp commented Feb 21, 2017

I haven't looked into the newly appeared issue, but if it's only about the lodash usage, I think I can rewrite this without using lodash. It was just convenient to have it.

hellp and others added 3 commits February 21, 2017 08:30
The non-stacked scattered chart as a basis reflects the download numbers
correctly. The stacked area chart before was a bit midleading. Both
approaches are a bit "noisy", so I added a 7-day moving average line for
each version number. This gives a higher-level overview and also makes it
easy to see when new versions overtake older ones in usage.

The colors were chosen to be colorblindness-friendly as well as having
a hot-cold metaphor.
@carols10cents carols10cents force-pushed the moving-avg-download-graph branch from 3d02252 to fae4553 Compare February 21, 2017 13:30
@carols10cents
Copy link
Member

Turns out it was my fault :) I just rebased and force pushed to pick up this change: #562

I think lodash is convenient too, seems fine. Let's see if this goes green.

@carols10cents
Copy link
Member

Yaaay, tests are green!!! 🍏 📗 💚 🥗 Tested it out locally and it's looking awesome! Thank you for your patience!!

@durka
Copy link
Contributor

durka commented Feb 22, 2017

Just my two cents: I saw the new graphs and I thought something was broken. Now reading this I like the moving average idea, but I think it needs to be indicated in the legend somehow.

@durka
Copy link
Contributor

durka commented Feb 22, 2017

I would even suggest removing the noisy dots. Or have a toggle to switch between averaged and raw data.

@carols10cents
Copy link
Member

carols10cents commented Feb 22, 2017 via email

@bstrie
Copy link

bstrie commented Feb 27, 2017

Loving the color selection and the use of a rolling average. Agreed with @durka that the dots initially made me think that something was broken, and I'd second a PR to remove them from the main graph (though I think it's useful to retain the raw data in the hover).

@carols10cents
Copy link
Member

@bstrie could you make your comment on #567 too please? thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants