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 axis-reversed stat_ecdf #3832

Closed
wants to merge 60 commits into from
Closed

Add axis-reversed stat_ecdf #3832

wants to merge 60 commits into from

Conversation

jgjl
Copy link
Contributor

@jgjl jgjl commented Feb 23, 2020

Add an option to reverse axis y->x instead of x->y to acomodate tail-latency plots, i.e.: https://github.com/ixy-languages/ixy-languages/blob/master/img/latency-hdr-hist-1.png (context: https://github.com/ixy-languages/ixy-languages).
In this kind of plot the probability is plotted along the horizontal axis while the values are plotted along the vertical axis. Hence, in comparison to the default stat_ecdf function, the axis are reversed.
This specific step could also be achived by applying coord_flip. However, in this case the reversed-logarithmic transformation of the x-axis cannot be applied. Hence, in my understanding, switching axis in stat_ecdf is the only way to get there.

I will supply the complementary transformation object for the x-axis separately outside of the ggplot2 package.

The proposed implementation is a little simplistic in that it just adds a version where input and output variables are reversed. An alternative would be to allow the input and output variables to be freely choosen. I am happy to supply such a version if it would be preferrable.

thomasp85 and others added 30 commits January 8, 2020 15:37
Add an option to reverse axis y->x instead of x->y to acomodate tail-latency plots, i.e.: https://github.com/ixy-languages/ixy-languages/blob/master/img/latency-hdr-hist-1.png.
topepo and others added 23 commits March 3, 2020 09:31
… from data (tidyverse#3789)

* improve lims docs: add warning about values outside of limits being removed from data, references tidyverse#3752

* place lim warning in first paragraph, references tidyverse#3752

* rerun roxygen, references tidyverse#3752
* testing

* experimenting

* update annotation_logticks

let ticks be outside of plot

* fix "outside" option

* formatting text

* news file update

* news file update

* news.md update

* reconcile News.md with recent updates

* format FALSE
…#3837)

* Let check_nondata_cols allow Vector class

* Remove as_tibble from as_gg_data_frame

* Add NEWS.md bullet

* Rephrase news bullet
Add an option to reverse axis y->x instead of x->y to acomodate tail-latency plots, i.e.: https://github.com/ixy-languages/ixy-languages/blob/master/img/latency-hdr-hist-1.png.
@jgjl
Copy link
Contributor Author

jgjl commented Mar 9, 2020

stat_ecdf now uses the now aes switching scheme, for which I added a test based on the test for stat_density. Furthermore, the branch was rebased on top of master.

@clauswilke
Copy link
Member

@jgjl Just an FYI: There's no need to rebase. Merge is fine. We squash-merge all PRs anyways.

@jgjl
Copy link
Contributor Author

jgjl commented Mar 11, 2020

@jgjl Just an FYI: There's no need to rebase. Merge is fine. We squash-merge all PRs anyways.

Should I create a new PR without the rebase?

@clauswilke
Copy link
Member

No, no need to change this now. I just wanted to point out that rebasing is not necessary, since it can be a lot of unnecessary extra work.

@clauswilke
Copy link
Member

@jgjl I think you'll have to create a new PR. This PR now lists 60 commits and 49 files changed and it just looks like things went wrong somewhere along the way. Also, I suggest you open an issue first describing the feature you'd like to add, and then you can make a new PR implementing that feature.

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.

None yet

10 participants