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

Fix img axes #4088

Merged
merged 3 commits into from
Feb 16, 2022
Merged

Fix img axes #4088

merged 3 commits into from
Feb 16, 2022

Conversation

t-bltg
Copy link
Member

@t-bltg t-bltg commented Feb 2, 2022

Fix #4087.

using Plots

png(plot(Gray.(randn(10,15))), "foo")

foo

@t-bltg t-bltg force-pushed the fix_img branch 3 times, most recently from 7620da6 to b5317f2 Compare February 2, 2022 15:41
@t-bltg t-bltg added the bug label Feb 2, 2022
@t-bltg
Copy link
Member Author

t-bltg commented Feb 3, 2022

@sefffal, I assumed that the labels will suit you, the alternatives are (horizontal axis as example).

  • label vertices 0:15 (PR, distance from border)
  • labels vertices 1:16
  • labels cells 1:15

Labelling n cells centers is strictly equivalent to labelling n+1 vertices, so this was the easiest fix. But if that doesn't suit you, we should find a way to label cell centers as with plotly (to me this is only a matter of personal taste), if that's even possible (not sure it's doable easily).

@sefffal
Copy link
Contributor

sefffal commented Feb 3, 2022

Thanks @t-bltg for looking at this!
My preference would definitely be to label the cell centres, 1:15. At least for most applications in astronomy we like to think of the indices as pointing to the centres of the pixels.
But if there is another convention that is more widely used, I can work around whatever you choose. Thank you!

@t-bltg
Copy link
Member Author

t-bltg commented Feb 3, 2022

Thanks for your reply.

What we could do right now is fix the current which is obviously wrong with 0:15, but keep #4087 opened until someone finds a more decent solution or a hack for centering ticks in GR.

@t-bltg
Copy link
Member Author

t-bltg commented Feb 6, 2022

@BeastyBlacksmith, whats is your opinion on this ? There is no point opening a PR in https://github.com/JuliaPlots/PlotReferenceImages.jl if this is not the fix which is going to get in.

@sefffal
Copy link
Contributor

sefffal commented Feb 6, 2022

A second reason to prefer labeling cells starting at 1 is that it matches nicely with Julia indexing. For example, you can access the cell at position 1,1 just by accessing arr[1,1]. Since Julia uses 1 based indexing I think this is more natural.

As an aside, how does this work for heatmaps? Can the tick positioning code from there be used for images as well?

@t-bltg
Copy link
Member Author

t-bltg commented Feb 6, 2022

As an aside, how does this work for heatmaps? Can the tick positioning code from there be used for images as well?

Looks like heatmap is centered at least for GR: https://docs.juliaplots.org/stable/generated/gr/#gr-ref43

@BeastyBlacksmith
Copy link
Member

I think we should make this consistent with heatmap

@t-bltg
Copy link
Member Author

t-bltg commented Feb 16, 2022

using Plots
img = randn(10, 15)
png(plot(Gray.(img), xticks=1:15, yticks=1:10), "foo")

hm_img_2

png(plot(Gray.(img)), "bar")

hm_img_3

@sefffal, @BeastyBlacksmith, ok now ?

@sefffal
Copy link
Contributor

sefffal commented Feb 16, 2022

Beautiful!

@codecov
Copy link

codecov bot commented Feb 16, 2022

Codecov Report

Merging #4088 (a7ea05b) into master (88cd04b) will not change coverage.
The diff coverage is 50.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4088   +/-   ##
=======================================
  Coverage   62.20%   62.20%           
=======================================
  Files          29       29           
  Lines        7315     7315           
=======================================
  Hits         4550     4550           
  Misses       2765     2765           
Impacted Files Coverage Δ
src/axes.jl 84.37% <ø> (ø)
src/examples.jl 63.63% <ø> (ø)
src/recipes.jl 58.49% <50.00%> (ø)
src/backends/gr.jl 88.27% <0.00%> (-0.09%) ⬇️
src/types.jl 52.17% <0.00%> (+4.34%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 88cd04b...a7ea05b. Read the comment docs.

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

Successfully merging this pull request may close these issues.

[BUG] image tick positions drifting/ misplaced
3 participants