Add Slycot exception and warning classes#117
Add Slycot exception and warning classes#117roryyorke merged 30 commits intopython-control:masterfrom
Conversation
repagh
left a comment
There was a problem hiding this comment.
Forcing a "None" positional parameter looks a bit ugly to me. For the rest, I like this initiative.
|
Ow. That went quicker than I thought. It seems I was able to push on your branch, if hope you like the modification, if not, ignore/revert. One of the advantages is that this reduces the number of lines; fewer lines - better coverage. Look, nice improvement: |
|
I like it, but right now the raise from docstring always raises |
|
I think I accidentally did a wrong force-push, trying to fix the loose ampersand. :-( |
|
Yeah, you discarded my edits. No worries, will fix that in my next commit. Probably need to rebase a little bit. |
It's cheating though because we now parse stuff that is not counted by coverage. ;) |
|
Nice work, you lifted it to a whole new level. Some additional points
|
|
Yes the original parsing check did not work anymore as the new parser does not return the complete list. Will fix the commit messages in the next rebase, where I also fix your "merge" commit. Right now I am getting gihub connection errors on git. I am working on extending this to "Warns" also. There are quite a few routines that use the info parameter for warnings but want to return partial results. |
exception parsing code
repagh
left a comment
There was a problem hiding this comment.
Looks good. However, I think we should raise a catch-all exception when we don't find the (non-zero) info value. Otherwise a (future) programming mistake might lead to someone happily accepting erroneous results.
- reintroduce catch all - syntax change: parse variables info and iwarn directly instead of e.info - SlycotWarning as also an iwarn attribute - update all the docstrings of functions that use the Raises and Warns sections - clean functions - update tests
repagh
left a comment
There was a problem hiding this comment.
looks good. I remarked some final issues
catch all unhandled warnings clean imports
|
Looks perfect now. Any objections to a merge? |
|
As soon as you said it's perfect, I broke it ;) Fixed now. With all the numpydoc changes, this is hard to review. Let's give @roryyorke a little bit of time to see if he has any objections. |
roryyorke
left a comment
There was a problem hiding this comment.
This looks great, thanks for all the work.
Besides #99, I image it goes a fair way to addressing #100.
_parse_docsection is a quite complex, but it's an internal tool, so that's fine.
I skimmed over the changes to analysis, synthesis, and math; it generally looks much tidier, and less code is always good.
|
So, time to merge and focus on the release before we get new ideas? ;) |
As proposed in #99, I am introducing the new Exceptions
SlycotError,SlycotParameterErrorandSlycotArithmeticError. So to never forget the.infoattribute again!Fixes #99