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

[FR] Option to widen manual limits #3339

Closed
lllusion3418 opened this issue Mar 5, 2021 · 8 comments · Fixed by #3552
Closed

[FR] Option to widen manual limits #3339

lllusion3418 opened this issue Mar 5, 2021 · 8 comments · Fixed by #3552
Labels
enhancement improving existing functionality

Comments

@lllusion3418
Copy link

lllusion3418 commented Mar 5, 2021

Sorry if I missed a previous issue.

Plots.jl version: 1.10.6

In this case I tried using ylims=(0,Inf) to indicate that i want the y-axis to start at 0 (because here the area under the blue curve somewhat matters):

This results in points at the maximum being cut off in the middle (and even the zero at the bottom isn't that pretty)! Because I couln't figure out another way except to calculate the maximum and use that times some factor (which I couldn't even do easily since they are a result of scatterhist()), I added plot!([0],[0], markersize=0, primary=false) to trick Plots.jl into thinking there was a point at y=0. This results in a somewhat nicer plot like this:

I get that lims are supposed to specify exact limits but I would think that the widen parameter would accomplish exactly what I want. Instead it gets ignored when custom limits are passed since it defaults to true (whereas if you did want the current behaviour you would also have had the option to set widen=false). Since it's probably a bit late to change that, could there at least be an option like widen = :force? If that's an option, it seems simple enough that I could implement it.

Another workaround I just found is using ylims() - adjusting that and passing it back in - but even that seems a bit complicated and hard to discover (is there a reason why there can't be some page just documenting all Plots.jl functions, or have I just somehow missed it? Examples are great but {x,y}lims() and e.g. mp4() don't seem to be mentioned anywhere).

Easy way to reproduce (with any backend):

scatter([1,2,3], [0,2,1], ylims=(0,Inf), legend=:none)

Edit: fwiw, this seems to work: ylims!(Plots.widen(ylims()...)) (unless you're using an logarithmic scale)

@fredcallaway
Copy link
Contributor

fredcallaway commented Mar 8, 2021

Wow what a coincidence, I came here to post about this exact issue and it's already the most recent one!

I would put in a vote that widening would happen by default (i.e. not requiring widen=:force). Not respecting widen when passing limits is really just a bug, and I don't think backwards compatibility concerns should prevent fixing it.

@BeastyBlacksmith BeastyBlacksmith added the enhancement improving existing functionality label Jun 7, 2021
@BeastyBlacksmith
Copy link
Member

I think this is reasonable. I'd appreciate if you are up to make a PR. PRs to improve the documentation are also welcome!

fredcallaway added a commit to fredcallaway/Plots.jl that referenced this issue Jun 7, 2021
Previously, the limits were not widened when passing a tuple or
:round to lims (ignoring the widen argument). Fixes JuliaPlots#3339
@fredcallaway
Copy link
Contributor

Given that someone had intentionally implemented default_should_widen to not widen when passing limits, it might be worth checking with other maintainers before accepting this. It's ultimately a subjective decision, and major plotting packages disagree on the correct behavior (matplotlib doesn't widen, ggplot does).

We can fall back on the widen=:force option if people don't agree with changing the default behavior.

@lllusion3418
Copy link
Author

Personally I would prefer always defaulting to widen=true as suggested as well, but if backwards compatibility is a concern, another way might be to keep the current behavior as long as widen isn't passed explicitly (change the default to :auto or something) and always respect true/false if passed explicitly. I.e.:

  • plot() == plot(widen = :auto) -> does widen
  • plot(ylims = (a, b)) == plot(ylims = (a, b), widen = :auto) -> doesn't widen
  • plot(widen = true) -> always widens regardless of limits
  • plot(widen = false) -> never widens

Technically this would also be a breaking change, but why would anyone pass widen = true but not actually expect it to so.

@BeastyBlacksmith
Copy link
Member

I like that a lot

@BeastyBlacksmith
Copy link
Member

BeastyBlacksmith commented Jun 7, 2021

Actually what I really would like is a new axis attribute lets say limits_modifiers where I can mix and match like

julia> plot(1:5) |> Plots.xlims # still widen by default
(0.88, 5.12)

julia> plot(1:5, xlimits_modifiers = :round) |> Plots.xlims # in this case same as `limits_modifiers = false/nothing/:none`
(1.0, 5.0)

julia> plot(1:5, xlimits_modifiers = (:widen, :round) |> Plots.xlims
(0.0, 6.0)

julia> plot(1:5, xlimits_modifiers = (:symmetric, :widen) |> Plots.xlims
(-5.12, 5.12)

But I'm not sure we can do that non-breaking.

@fredcallaway
Copy link
Contributor

Agreed. Something like that would be more consistent with Pots.jl style. However, this would be a substantially more involved change. Thus, I think it makes sense to merge my existing pull request, which implements @lllusion3418's :auto suggestion.

@BeastyBlacksmith
Copy link
Member

Agreed. I'll put this in a seperate issue on the 2.0 milestone

BeastyBlacksmith added a commit that referenced this issue Jun 7, 2021
* Respect :widen when passing :lims

Previously, the limits were not widened when passing a tuple or
:round to lims (ignoring the widen argument). Fixes #3339

* Add default widen=:auto, which matches default before 43e9a34

* If widen is a Bool, it always determines whether widening occurs.
* If widen is :auto (the defualt), widening occurs for appropriate
  seriestypes, unless lims were set manually

* Update test/test_axes.jl

Co-authored-by: Simon Christ <[email protected]>

* fix docs for lims and widen interaction

* Update .zenodo.json

* Update test/test_axes.jl

xlims != ylims

Co-authored-by: Simon Christ <[email protected]>

Co-authored-by: Simon Christ <[email protected]>
daschw pushed a commit to daschw/Plots.jl that referenced this issue Jun 13, 2021
* Respect :widen when passing :lims

Previously, the limits were not widened when passing a tuple or
:round to lims (ignoring the widen argument). Fixes JuliaPlots#3339

* Add default widen=:auto, which matches default before 43e9a34

* If widen is a Bool, it always determines whether widening occurs.
* If widen is :auto (the defualt), widening occurs for appropriate
  seriestypes, unless lims were set manually

* Update test/test_axes.jl

Co-authored-by: Simon Christ <[email protected]>

* fix docs for lims and widen interaction

* Update .zenodo.json

* Update test/test_axes.jl

xlims != ylims

Co-authored-by: Simon Christ <[email protected]>

Co-authored-by: Simon Christ <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement improving existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants