Improve int indexing depwarn msg

[lrennels]

This PR addresses #690, and improves the error message for the deprecation warning associated with integer indexing. Since the Stacktrace will change in each call, the improvement for now prints out the stacktrace() stack frame line by line instead of as a text block, ie. the following.

The component printout could still be quite long, however, but at least printed nicely.

┌ Warning: Indexing with getindex into a TimestepArray with Integer(s) is deprecated, please index with a TimestepIndex(index::Int) instead ie. instead of t[2] use t[TimestepIndex(2)]
│  
│ _get_stacktrace_string() at time_arrays.jl:62
│ _throw_int_getindex_depwarning at time_arrays.jl:71 [inlined]
│ getindex(::TimestepArray{FixedTimestep{2000,1,LAST} where LAST,Int64,1,1}, ::Int64) at time_arrays.jl:209
│ top-level scope at REPL[68]:1
│ eval(::Module, ::Any) at boot.jl:331
│ eval_user_input(::Any, ::REPL.REPLBackend) at REPL.jl:86
│ macro expansion at REPL.jl:118 [inlined]
│ (::REPL.var"#26#27"{REPL.REPLBackend})() at task.jl:358
│   caller = top-level scope at REPL[68]:1
└ @ Core REPL[68]:1

[lrennels]

@davidanthoff @ckingdon95 should we leave the whole stacktrace, or nab just a part of it? If I take just the third line of the stack trace I think that will always be the line number of the expanded component, but that seems a little bit fragile.

[codecov]

Codecov Report

Merging #694 into master will decrease coverage by 0.03%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master     #694      +/-   ##
==========================================
- Coverage   79.19%   79.16%   -0.04%     
==========================================
  Files          39       39              
  Lines        2841     2851      +10     
==========================================
+ Hits         2250     2257       +7     
- Misses        591      594       +3     

Flag	Coverage Δ
#unittests	79.16% <66.66%> (-0.04%)	⬇️

Impacted Files	Coverage Δ
src/core/time_arrays.jl	82.52% <66.66%> (-0.49%)	⬇️

---

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 0fbe2d5...24aebc7. Read the comment docs.

[corakingdon]

@lrennels yeah I think if possible we could cut the first two or three lines

[corakingdon]

Although I think the whole component instance type might still get printed out on the "macro expansion" line. Have you tried it with the mimi-next branch of PAGE? does this make it any less long?

[corakingdon]

actually maybe we can implement something even sneakier: if the error message is coming from inside of a run_timestep function (which 99% of these should be) then could your _throw_int_getindex_depwarning function check for that? aka I think there will always be a line that starts with

macro expansion at EquityWeighting.jl:116 [inlined]

followed by a line like:

run_timestep_MimiPAGE2009_EquityWeighting(::Mimi.ComponentInstanceParameters{NamedTuple . . .
# this is the really long one because the function handle contains all the component type info

So maybe we can drop any lines that start with "run_timestep" so that it would just return the previous line, the one that actually says the line number in the component code?

[lrennels]

@ckingdon95 yea that's exactly what I was thinking of doing, return the first line that starts with macro expansion I just wasn't sure if that seemed too fragile

[corakingdon]

I think that makes sense to only return that line. But then if there aren't any lines that start with "macro expansion..." return the whole thing

[lrennels]

@ckingdon95 my testing showed some other cases here, maybe we can shorten the run_timestep lines instead? Something like

# Helper to print stacktrace for the integer indexing deprecatino warning
function _get_stacktrace_string()
  s = ""
    for line in stacktrace()
	  line_string = string(line)
	  if startswith(line_string, "run_timestep") # shorten calls to run_timestep
	      l = "run_timestep function call"
	  else
	      l = line_string
	  end
          s = string(s, l, "\n")
    end
    return s
end

[corakingdon]

@lrennels there are definitely tests in the test files where this arises outside of a "run_timestep" function, but I meant that in terms of actual users I think this warning will 99% be coming from "run_timestep" functions.

[corakingdon]

My preference would be to just return s here. If it hits a "run_timestep" line, then the previous line would be the "macro expansion" line which has the actual component line number info of where the error is happening

[lrennels]

@ckingdon95 sounds good to me, done!