PoisotLab / SimpleSDMLayers.jl

Simple layers for species distribution modeling and bioclimatic data
https://docs.ecojulia.org/SimpleSDMLayers.jl/stable/
MIT License
19 stars 2 forks source link

Add indentation in show method #98

Closed gabrieldansereau closed 3 years ago

gabrieldansereau commented 3 years ago

What the pull request does

I really like the show methods but I think we should add some indentation before the coordinates on the 2nd and 3rd lines to differentiate them from the layer type on the 1st line. Especially for Vector{SimpleSDMLayer} with multiple elements, where it can get hard to see the different layers.

I tried a couple things, so I'll commit each one separately and add a comment showing the result.

Here's the output before the changes in this PR:

julia> layer = SimpleSDMPredictor(WorldClim, BioClim, 1)
SDM predictor → 1080×2160 grid with 808053 Float32-valued cells
Latitudes       (-89.91666666666667, 89.91666666666667)
Longitudes      (-179.91666666666666, 179.91666666666666)

julia> layers = SimpleSDMPredictor(WorldClim, BioClim, 1:2)
2-element Vector{SimpleSDMPredictor{Float32}}:
 SDM predictor → 1080×2160 grid with 808053 Float32-valued cells
Latitudes       (-89.91666666666667, 89.91666666666667)
Longitudes      (-179.91666666666666, 179.91666666666666)
 SDM predictor → 1080×2160 grid with 808053 Float32-valued cells
Latitudes       (-89.91666666666667, 89.91666666666667)
Longitudes      (-179.91666666666666, 179.91666666666666)

Type of change

Please indicate the relevant option(s)

Checklist

gabrieldansereau commented 3 years ago

Option 1

Adds two spaces before the coordinates. Elements in the vector of layers are already indented by one space, so there's still a difference of one space.

julia> layer
SDM predictor → 1080×2160 grid with 808053 Float32-valued cells
  Latitudes     (-89.91666666666667, 89.91666666666667)
  Longitudes    (-179.91666666666666, 179.91666666666666)

julia> layers
2-element Vector{SimpleSDMPredictor{Float32}}:
 SDM predictor → 1080×2160 grid with 808053 Float32-valued cells
  Latitudes     (-89.91666666666667, 89.91666666666667)
  Longitudes    (-179.91666666666666, 179.91666666666666)
 SDM predictor → 1080×2160 grid with 808053 Float32-valued cells
  Latitudes     (-89.91666666666667, 89.91666666666667)
  Longitudes    (-179.91666666666666, 179.91666666666666)
codecov-commenter commented 3 years ago

Codecov Report

Merging #98 (268c807) into master (24aed45) will decrease coverage by 0.42%. The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #98      +/-   ##
==========================================
- Coverage   80.45%   80.02%   -0.43%     
==========================================
  Files          24       24              
  Lines         752      756       +4     
==========================================
  Hits          605      605              
- Misses        147      151       +4     
Flag Coverage Δ
unittests 80.02% <0.00%> (-0.43%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/lib/overloads.jl 86.92% <0.00%> (-2.76%) :arrow_down:

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 24aed45...268c807. Read the comment docs.

gabrieldansereau commented 3 years ago

Option 2

Same layout as before for the single layer, but with a 3 argument show with a MIME type. This way, the vector of layers can be shown differently.

julia> layer
SDM predictor → 1080×2160 grid with 808053 Float32-valued cells
  Latitudes     (-89.91666666666667, 89.91666666666667)
  Longitudes    (-179.91666666666666, 179.91666666666666)

julia> layers
2-element Vector{SimpleSDMPredictor{Float32}}:
 SDM predictor → 1080×2160 grid with 808053 Float32-valued cells
 SDM predictor → 1080×2160 grid with 808053 Float32-valued cells

What I really like with that option is that it doesn't overflow the REPL view with Arrays of multiple layers. It shows only the number of elements which fits in the view, as with other Array types. Of course it hides the coordinates, but I'm not sure those are useful on a multiple layers view. And the grid size is still displayed, so that can be an indicator if layers aren't equivalent.

julia> SimpleSDMPredictor(WorldClim, BioClim, 1:19)
19-element Vector{SimpleSDMPredictor{Float32}}:
 SDM predictor → 1080×2160 grid with 808053 Float32-valued cells
 SDM predictor → 1080×2160 grid with 808053 Float32-valued cells
 SDM predictor → 1080×2160 grid with 808053 Float32-valued cells
 ⋮
 SDM predictor → 1080×2160 grid with 808053 Float32-valued cells
 SDM predictor → 1080×2160 grid with 808053 Float32-valued cells
 SDM predictor → 1080×2160 grid with 808053 Float32-valued cells
gabrieldansereau commented 3 years ago

I was going to suggest a Base.show(io::IO, ::MIME"text/plain", layers::Vector{T}) method, but it's complicated and doesn't give a better result.

My preference would be for Option 2.

@tpoisot any thoughts on this?

tpoisot commented 3 years ago

I like option 2 (a different method for arrays).

Feel free to merge!