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

conditionally require FreeType, FileIO, Unitful and ImageInTerminal through weak deps or Requires #332

Merged
merged 10 commits into from
Feb 8, 2023

Conversation

t-bltg
Copy link
Member

@t-bltg t-bltg commented Feb 4, 2023

Fix #331.
Saving as .png is mostly for Plots, and not the primary target of UnicodePlots so we now make FreeType and FileIO optional.
It's also likely Unitful is already loaded when you are plotting quantities, so it's unlikely that making it a weak dependency will break anyone's workflow (glue code) even if this is technically breaking ?

I guess this will require a major bump ... Thoughts ?

As mentioned on slack, making glue package code optional seems to be non breaking in this case so a minor bump is probably sufficient.

I think I'll wait the 1.9.0-beta4 release to merge this PR for the backport of JuliaLang/julia#48352.

latency improvements: ~2s, about 32% gain

julia 1.9.0-beta3

pr

julia> @time @time_imports using UnicodePlots
     30.5 ms  Preferences
      0.3 ms  SnoopPrecompile
      0.9 ms  Statistics
      3.3 ms  StaticArraysCore
    885.5 ms  StaticArrays
     61.0 ms  FixedPointNumbers
    140.2 ms  ColorTypes 21.49% compilation time (100% recompilation)
     38.9 ms  Crayons
      1.2 ms  DataAPI
      0.2 ms  Compat
     10.2 ms  OrderedCollections
    106.7 ms  DataStructures
      0.5 ms  SortingAlgorithms
     12.6 ms  Missings
      1.4 ms  DocStringExtensions
      7.1 ms  IrrationalConstants
      0.4 ms  LogExpFunctions
      0.3 ms  StatsAPI
     33.6 ms  StatsBase
     81.4 ms  MarchingCubes
      0.3 ms  Reexport
    302.1 ms  Colors
    117.0 ms  ChainRulesCore
      0.7 ms  OpenLibm_jll
      0.3 ms  JLLWrappers
      6.7 ms  OpenSpecFun_jll 90.29% compilation time
     23.9 ms  SpecialFunctions
      0.3 ms  TensorCore
    253.4 ms  ColorVectorSpace 4.99% compilation time (100% recompilation)
     11.1 ms  ColorSchemes
      0.3 ms  NaNMath
      1.7 ms  Contour
   1781.9 ms  UnicodePlots
  4.167651 seconds (5.65 M allocations: 371.606 MiB, 6.49% gc time, 5.21% compilation time: 20% of which was recompilation)

master

julia> @time @time_imports using UnicodePlots
     34.1 ms  Preferences
      0.4 ms  SnoopPrecompile
      1.1 ms  Statistics
      4.5 ms  StaticArraysCore
   1060.5 ms  StaticArrays
     67.0 ms  FixedPointNumbers
    133.2 ms  ColorTypes 22.10% compilation time (100% recompilation)
     36.6 ms  Crayons
      1.8 ms  ConstructionBase
   1233.0 ms  Unitful
      1.9 ms  DataAPI
      0.2 ms  Compat
     11.1 ms  OrderedCollections
    111.3 ms  DataStructures
      0.6 ms  SortingAlgorithms
     15.0 ms  Missings
      1.6 ms  DocStringExtensions
      7.2 ms  IrrationalConstants
      0.4 ms  LogExpFunctions
      0.3 ms  StatsAPI
     36.0 ms  StatsBase
     86.5 ms  MarchingCubes
      0.4 ms  Reexport
    313.4 ms  Colors
    119.2 ms  ChainRulesCore
      0.5 ms  OpenLibm_jll
      0.4 ms  JLLWrappers
      6.2 ms  OpenSpecFun_jll 87.53% compilation time
     25.9 ms  SpecialFunctions
      0.4 ms  TensorCore
    267.2 ms  ColorVectorSpace 4.17% compilation time (100% recompilation)
     12.3 ms  ColorSchemes
      0.4 ms  Requires
      0.3 ms  NaNMath
      1.9 ms  Contour
    242.5 ms  FileIO
      1.0 ms  Bzip2_jll
      0.3 ms  Zlib_jll
      0.6 ms  FreeType2_jll
      6.6 ms  CEnum
      8.7 ms  FreeType
   2003.9 ms  UnicodePlots
  6.176219 seconds (7.55 M allocations: 486.762 MiB, 8.17% gc time, 3.75% compilation time: 18% of which was recompilation)

@t-bltg t-bltg added the latency label Feb 4, 2023
@codecov-commenter
Copy link

codecov-commenter commented Feb 4, 2023

Codecov Report

Base: 99.80% // Head: 99.80% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (e8c7928) compared to base (e221cee).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #332   +/-   ##
=======================================
  Coverage   99.80%   99.80%           
=======================================
  Files          32       34    +2     
  Lines        2036     2048   +12     
=======================================
+ Hits         2032     2044   +12     
  Misses          4        4           
Impacted Files Coverage Δ
src/interface/lineplot.jl 100.00% <ø> (ø)
src/interface/scatterplot.jl 91.66% <ø> (-1.02%) ⬇️
ext/FreeTypeExt.jl 100.00% <100.00%> (ø)
ext/ImageInTerminalExt.jl 100.00% <100.00%> (ø)
ext/UnitfulExt.jl 100.00% <100.00%> (ø)
src/UnicodePlots.jl 100.00% <100.00%> (ø)
src/common.jl 99.59% <100.00%> (-0.01%) ⬇️
src/graphics/imagegraphics.jl 100.00% <100.00%> (ø)
src/show.jl 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@t-bltg t-bltg force-pushed the weak branch 3 times, most recently from 6822761 to f7904d6 Compare February 4, 2023 20:57
@t-bltg t-bltg changed the title conditionally require FreeType and ImageInTerminal through weak deps or Requires conditionally require FreeType, FileIO and ImageInTerminal through weak deps or Requires Feb 4, 2023
@t-bltg t-bltg force-pushed the weak branch 2 times, most recently from 6856beb to 75cae03 Compare February 4, 2023 22:39
@t-bltg t-bltg changed the title conditionally require FreeType, FileIO and ImageInTerminal through weak deps or Requires conditionally require FreeType, FileIO, Unitful and ImageInTerminal through weak deps or Requires Feb 5, 2023
@t-bltg t-bltg force-pushed the weak branch 2 times, most recently from 320543d to 016edc7 Compare February 5, 2023 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Request: Leverage package extensions
2 participants