Skip to content
This repository has been archived by the owner on Dec 11, 2022. It is now read-only.

Add indentation in show method #98

Merged
merged 2 commits into from
May 14, 2021
Merged

Add indentation in show method #98

merged 2 commits into from
May 14, 2021

Conversation

gabrieldansereau
Copy link
Member

@gabrieldansereau gabrieldansereau commented May 14, 2021

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)

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ❇️ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📖 This change requires a documentation update

Checklist

  • The changes are documented
    • The docstrings of the different functions describe the arguments, purpose, and behavior
    • There are examples in the documentation website
  • The changes are tested
  • The changes do not modify the behavior of the previously existing functions
    • If they do, please explain why and how in the introduction paragraph
  • For new contributors - my name and information are added to .zenodo.json
  • The Project.toml field version has been updated
    • Change the last number for a v0.0.x series release, a bug fix, or a performance improvement
    • Change the middle number for additional features that do not break the current test suite (unless you fix a bug in the test suite)
    • Change the first number for changes that break the current test suite

@gabrieldansereau
Copy link
Member Author

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

This comment has been minimized.

@gabrieldansereau
Copy link
Member Author

gabrieldansereau commented May 14, 2021

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
Copy link
Member Author

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?

@gabrieldansereau gabrieldansereau marked this pull request as ready for review May 14, 2021 02:14
@gabrieldansereau gabrieldansereau requested a review from tpoisot May 14, 2021 02:14
@tpoisot
Copy link
Member

tpoisot commented May 14, 2021

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

Feel free to merge!

@gabrieldansereau gabrieldansereau merged commit c7a9d2e into master May 14, 2021
@gabrieldansereau gabrieldansereau deleted the gd/show branch May 14, 2021 12:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants