RootLocusResult recipe

[mzaffalon]

Add struct RootLocusResult and Plot recipe

[mzaffalon]

I am not familiar with Plots and its recipes system, so I just copy-pasted the rlocusplot's code. I would love to have somebody show me how to avoid code duplication.

[codecov]

Codecov Report

Merging #792 (42e54dc) into master (236866f) will increase coverage by 0.00%.
The diff coverage is 75.00%.

❗ Current head 42e54dc differs from pull request most recent head 80f7fb8. Consider uploading reports for the commit 80f7fb8 to get more accurate results

@@           Coverage Diff           @@
##           master     #792   +/-   ##
=======================================
  Coverage   97.13%   97.14%           
=======================================
  Files           4        4           
  Lines         314      315    +1     
=======================================
+ Hits          305      306    +1     
  Misses          9        9           

Impacted Files	Coverage Δ
src/root_locus.jl	97.77% <75.00%> (+0.02%)	⬆️

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

[baggepinnen]

Thanks for this PR!

Looking at the changes, it does not look like you have duplicated any code from rlocusplot? Since the tests pass, you have not broken the old functionality either so I think this looks good as it is :)

[mzaffalon]

Thank you for taking a look at the PR!

Yes, no more duplication: I figured out how to do it this morning.

There are two more changes I would like to discuss:

1. can the return type of getpoles(G, K::Number) https://github.com/JuliaControl/ControlSystems.jl/blob/master/src/root_locus.jl#L45 be the same as a that of getpoles(G, K::AbstractVector{T}) where {T<:Number}, that is a Matrix instead of an transposed matrix?
2. should the types of RootLocusResult fe81aa3#diff-10c6446e6ced8e95a649dc28592df86d16e33c92d7898a7d57d6a29ea5c36018R88 be tighter?

[baggepinnen]

can the return type of getpoles(G, K::Number)
ttps://github.com/JuliaControl/ControlSystems.jl/blob/master/src/root_locus.jl#L45 be the same as a that of getpoles(G, K::AbstractVector{T}) where {T<:Number}, that is a Matrix instead of an transposed matrix?

Yeah I guess. It shouldn't really matter for generic code, but this is typically not a performance sensitive function so the copy would probably not be problematic.

should the types of RootLocusResult fe81aa3#diff-10c6446e6ced8e95a649dc28592df86d16e33c92d7898a7d57d6a29ea5c36018R88 be tighter?

Do they need to be? Users are not expected to create this type themselves so guarding against the wrong field types is not required, and all fields of the struct are fully parameterized so there would be no performance benefit either.

[mzaffalon]

Do they need to be? Users are not expected to create this type themselves so guarding against the wrong field types is not required, and all fields of the struct are fully parameterized so there would be no performance benefit either.

That is true. It bothers me (purely aesthetically) because those fields are already defined in the code as vectors of ComplexF64.

[baggepinnen]

Well, you could make that restriction on the fields of the type if you want, I think the only difference would be that the type would print less verbosely :)

[mzaffalon]

You are right, there is no much gain. I did not change anything there.

I think the PR is complete: let me know if you have feedback.

[baggepinnen]

Thanks for the PR, a new release will be out shortly :)