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

Various ticks fixes (precision, log scale) #124

Merged
merged 9 commits into from
Dec 13, 2021

Conversation

t-bltg
Copy link
Member

@t-bltg t-bltg commented Dec 11, 2021

  1. Fix precision issues (fix optimize_ticks not working right for small values #114);
  2. Relax exponent integer constraint when computing log based ticks (fix Spurious "no strict ticks" warning for log10 scales #116, fix [BUG] Log scale plots raise warning and break subsequent non-log scale plots Plots.jl#3918).
  3. Coding style.
  4. Update CI script, using julia LTS 1.6 and latest 1.7 (following test on 1.6 also Plots.jl#3966).
  5. Drop julia versions < 1.6 for consistency with Plots.
  6. Bump minor version.
  7. Add formatting github action, as in Plots.

Xref: JuliaPlots/Plots.jl#3790, JuliaPlots/Plots.jl#3577, JuliaPlots/Plots.jl#3921.

Ran Plots.jl test suite without any apparent issue.

@t-bltg
Copy link
Member Author

t-bltg commented Dec 11, 2021

@BeastyBlacksmith, can you disable the AppVeyor CI action ? I don't think we'll use it anymore, and I don't have access to the settings of any JuliaPlots organization repo.

@t-bltg t-bltg closed this Dec 11, 2021
@t-bltg t-bltg reopened this Dec 11, 2021
@t-bltg t-bltg changed the title Various fixes Various ticks fixes (precision, log scale) Dec 11, 2021
Copy link
Member

@BeastyBlacksmith BeastyBlacksmith left a comment

Choose a reason for hiding this comment

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

In general I feel like this algorithm could try harder to show more than two ticks on log scales, but overall this is already an imrpovement

@t-bltg t-bltg merged commit b2c8065 into JuliaPlots:master Dec 13, 2021
@t-bltg t-bltg deleted the precision branch December 13, 2021 18:28
@ffreyer
Copy link
Contributor

ffreyer commented Dec 14, 2021

Looks like this messed up Makie's ticks in some cases.

With v1.0.15:

ticks = PlotUtils.optimize_ticks(0.25314125f0, 0.94421935f0, k_min = 4, k_max = 8)[1]
# [0.30000000000000004, 0.4, 0.5, 0.6000000000000001, 0.7000000000000001, 0.8, 0.9]
Showoff.showoff(ticks, :plain)
# ["0.3", "0.4", "0.5", "0.6", "0.7", "0.8", "0.9"]

With v1.1.0:

ticks = PlotUtils.optimize_ticks(0.25314125f0, 0.94421935f0, k_min = 4, k_max = 8)[1]
# Float32[0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.90000004]
Showoff.showoff(ticks, :plain)
# ["0.30000001", "0.40000001", "0.50000000", "0.60000002", "0.69999999", "0.80000001", "0.90000004"]

@t-bltg
Copy link
Member Author

t-bltg commented Dec 14, 2021

Ha, thanks for reporting, not surprising, will fix.

EDIT: Actually @ffreyer I'd expect optimize_ticks to return ticks with the same type as what was given as input for floats, why would it be otherwise ?

@ffreyer
Copy link
Contributor

ffreyer commented Dec 14, 2021

I'd expect that too. Maybe this should be fixed in Showoff or Makie?

@t-bltg
Copy link
Member Author

t-bltg commented Dec 14, 2021

I'd expect that too. Maybe this should be fixed in Showoff or Makie?

I'm not familiar with Showoff (I see lots of hardcoded Float64 in their algorithms), but the easiest would be to cast to Float64 in Makie:

$ grep -r optimize_ticks
src/basic_recipes/axis.jl:    scaled_ticks, mini, maxi = optimize_ticks(
src/basic_recipes/axis.jl:    scaled_ticks, mini, maxi = optimize_ticks(
src/makielayout/ticklocators/wilkinson.jl:    tickvalues, _ = PlotUtils.optimize_ticks(vmin, vmax;

, only 3 occurrences in Makie.

@t-bltg
Copy link
Member Author

t-bltg commented Dec 14, 2021

Seems like I introduced type instability in this PR, fix is underway ...

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