Skip to content

Conversation

lrennels
Copy link
Collaborator

This deprecation warning needs to be an error for v1.0.0, I noticed this when working on PAGE.

@lrennels lrennels added this to the v1.0.0 milestone Jul 14, 2020
@lrennels lrennels self-assigned this Jul 14, 2020
@codecov
Copy link

codecov bot commented Jul 14, 2020

Codecov Report

Merging #731 into master will decrease coverage by 0.01%.
The diff coverage is 62.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #731      +/-   ##
==========================================
- Coverage   80.64%   80.62%   -0.02%     
==========================================
  Files          38       38              
  Lines        2867     2844      -23     
==========================================
- Hits         2312     2293      -19     
+ Misses        555      551       -4     
Flag Coverage Δ
#unittests 80.62% <62.50%> (-0.02%) ⬇️
Impacted Files Coverage Δ
src/core/time.jl 94.36% <0.00%> (ø)
src/mcs/mcs_types.jl 79.16% <0.00%> (+3.16%) ⬆️
src/core/defcomp.jl 92.25% <100.00%> (+0.48%) ⬆️
src/core/model.jl 82.85% <100.00%> (-2.51%) ⬇️
src/core/time_arrays.jl 84.58% <100.00%> (-0.31%) ⬇️

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 640d7f6...7c84f07. Read the comment docs.

@lrennels lrennels requested a review from davidanthoff July 14, 2020 04:29
msg = "getindex method for `SimulationInstance` has been replaced with getdataframe function for consistency of return type, please use getdataframe instead"
Base.depwarn("$msg, $(stacktrace())", :getindex)
return sim_inst.results[model][(comp_name, datum_name)]
error("$msg")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
error("$msg")
error(msg)

:) Or don't use a local var at all and just pass the string directly to error.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All relics of older more complex messages from depreciations :) ... I’ll go through these today

Copy link
Collaborator

@corakingdon corakingdon left a comment

Choose a reason for hiding this comment

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

awesome, thanks for going through these Lisa!

@lrennels lrennels merged commit 2538cd2 into master Jul 15, 2020
@lrennels lrennels deleted the sim-error branch July 15, 2020 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants