Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove matrix size from StateSpace types #411

Merged
merged 3 commits into from
Jan 17, 2021

Conversation

baggepinnen
Copy link
Member

Solves #399 and supersedes #400

@olof3
Copy link
Contributor

olof3 commented Dec 12, 2020

Looks good, great to get rid of those fields. There were a lot of other fixes in #410, what about those?

@baggepinnen
Copy link
Member Author

I accidentally included tons of unrelated commits from my working branch. They might make their way into master eventually, but I have a few PRs waiting to be merged before I can submit the rest of the changes.

@olof3
Copy link
Contributor

olof3 commented Dec 12, 2020

Looked like there was a lot of useful stuff there :)

@baggepinnen baggepinnen reopened this Jan 16, 2021
@baggepinnen
Copy link
Member Author

baggepinnen commented Jan 17, 2021

Hmm, the failing test only fails on julia v1.5, not on julia v1.0 or nightly
https://github.com/JuliaControl/ControlSystems.jl/pull/411/checks?check_run_id=1714851690#step:6:251
I'll deactivate the test temporarily, it's a type-stability test and I do't believe the spurious failure on v1.5 but not on v1.6 is anything to worry about.

@codecov
Copy link

codecov bot commented Jan 17, 2021

Codecov Report

Merging #411 (550fd02) into master (6c54a25) will decrease coverage by 5.30%.
The diff coverage is 96.87%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #411      +/-   ##
==========================================
- Coverage   82.40%   77.09%   -5.31%     
==========================================
  Files          31       31              
  Lines        2824     2855      +31     
==========================================
- Hits         2327     2201     -126     
- Misses        497      654     +157     
Impacted Files Coverage Δ
src/types/StateSpace.jl 90.16% <96.87%> (-0.32%) ⬇️
src/pid_design.jl 32.57% <0.00%> (-45.38%) ⬇️
src/discrete.jl 40.54% <0.00%> (-43.72%) ⬇️
src/types/SisoTf.jl 25.00% <0.00%> (-25.00%) ⬇️
src/types/SisoTfTypes/SisoZpk.jl 60.00% <0.00%> (-21.11%) ⬇️
src/types/Lti.jl 50.00% <0.00%> (-7.15%) ⬇️
src/freqresp.jl 88.00% <0.00%> (-3.90%) ⬇️
src/utilities.jl 80.21% <0.00%> (-2.58%) ⬇️
src/timeresp.jl 91.33% <0.00%> (-2.21%) ⬇️
src/types/SisoTfTypes/SisoRational.jl 72.58% <0.00%> (-1.62%) ⬇️
... and 3 more

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 6c54a25...550fd02. Read the comment docs.

@baggepinnen baggepinnen merged commit 11f7438 into JuliaControl:master Jan 17, 2021
@baggepinnen baggepinnen deleted the rmfields2 branch January 17, 2021 06:26
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.

2 participants