Skip to content

One limit #908

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

Merged
merged 5 commits into from
Feb 25, 2014
Merged

One limit #908

merged 5 commits into from
Feb 25, 2014

Conversation

jimhester
Copy link
Contributor

This is a up to date rebase of #684, it addresses issue #557.

@hadley
Copy link
Member

hadley commented Feb 24, 2014

Thanks! You can be the first to try out my new PR checklist:

Can you please :

  • Motivate the change in one paragraph, and include it in NEWS.
    Reference this issue number and thank yourself.
  • Check pull request only includes relevant changes.
  • Use the official style.
  • Update documentation and re-run roxygen2
  • Add minimal example, if new graphical feature

@jimhester
Copy link
Contributor Author

I added info in news and updated some documentation. I am not sure this needs a graphic example, but if you think it should have one I can create one.

@@ -7,6 +7,11 @@ ggplot2 0.9.3.1.99
* `ggpcp()`, `ggfluctuation()`, `ggmissing()`, `ggstructure()`, and
`ggorder()` are now defunct and have been removed.

MINOR FEATURES
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove this heading and put your news item at the top?

@jimhester
Copy link
Contributor Author

Not sure if you get notified if I just add a commit so I wanted to comment and let you know I fixed the news issue you mentioned.

@hadley
Copy link
Member

hadley commented Feb 25, 2014

Thanks (I'm not). It looks good to go - can you please rebase/merge (probably just a conflict from NEWS) and then I'll accept.

Implements setting of only one limit of a continuous scale by specifying
NA for the limit which you don't want to set.
@jimhester
Copy link
Contributor Author

Done, let me know if everything looks good!

@hadley
Copy link
Member

hadley commented Feb 25, 2014

Looks like https://travis-ci.org/hadley/ggplot2/builds/19600088 failed - can you take a look please?

The range of mtcars wt data is from 1.513 to 5.424, so the previous example was
causing an error because there were no points to obtain a range from.
@jimhester
Copy link
Contributor Author

The error was occurring because the lower bound in the example was excluding all the points in the dataset so there was no way to calculate the upper bound. I fixed the example, I don't know if we should do something to address this error however...

hadley added a commit that referenced this pull request Feb 25, 2014
@hadley hadley merged commit 52baf83 into tidyverse:master Feb 25, 2014
@hadley
Copy link
Member

hadley commented Feb 25, 2014

I'd be happy to accept a pull request that gave a better error message in that case :)

@cmorty
Copy link

cmorty commented Mar 28, 2014

@jimhester How difficult do you think it is to implement this for the x/ylim for coord_cartesian? I'm no R expert and want to know whether its worth the trouble digging into it.

@jimhester
Copy link
Contributor Author

@cmorty It is definitely possible and would be useful. As to how hard it would be to implement it should be a fairly small change once you find the proper place to make the change. Finding that place might be challenging however, I would start with trying it out, when it errors use traceback() to see where it breaks.

@l-d-s
Copy link

l-d-s commented Nov 30, 2015

@cmorty Similarly for expand.

@lock
Copy link

lock bot commented Jan 18, 2019

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/

@lock lock bot locked and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants