Add Slycot exception and warning classes #117

bnavigator · 2020-05-03T22:08:46Z

As proposed in #99, I am introducing the new Exceptions SlycotError, SlycotParameterError and SlycotArithmeticError. So to never forget the .info attribute again!

Fixes #99

repagh

Forcing a "None" positional parameter looks a bit ugly to me. For the rest, I like this initiative.

slycot/analysis.py

repagh · 2020-05-05T14:33:10Z

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:

coverage/coveralls — Coverage increased (+25.1%) to 68.396%

bnavigator · 2020-05-05T15:32:58Z

I like it, but right now the raise from docstring always raises SlycotParameterError even when it should be SlycotArithmeticError. Working on this right now

repagh · 2020-05-05T20:09:17Z

I think I accidentally did a wrong force-push, trying to fix the loose ampersand. :-(

bnavigator · 2020-05-05T20:22:42Z

Yeah, you discarded my edits. No worries, will fix that in my next commit. Probably need to rebase a little bit.

bnavigator · 2020-05-06T09:19:02Z

Look, nice improvement:

coverage/coveralls — Coverage increased (+25.1%) to 68.396%

It's cheating though because we now parse stuff that is not counted by coverage. ;)

repagh · 2020-05-06T10:32:55Z

Nice work, you lifted it to a whole new level.

Some additional points

I see you removed the parsing check from test_sb.py. I worked on an easy routine to verify that a docstring can be parsed for the exceptions that we think are in there. That would offset the "cheating"?
I am getting these complaints from git that the recent commit messages have a & and a <, fix or ignore?

bnavigator · 2020-05-06T11:22:54Z

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

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.

slycot/exceptions.py

slycot/math.py

- 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

looks good. I remarked some final issues

slycot/math.py

slycot/exceptions.py

slycot/synthesis.py

catch all unhandled warnings clean imports

repagh · 2020-05-07T15:30:59Z

Looks perfect now. Any objections to a merge?

bnavigator · 2020-05-07T18:29:48Z

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.

slycot/math.py

roryyorke

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.

bnavigator · 2020-05-09T18:53:09Z

So, time to merge and focus on the release before we get new ideas? ;)

bnavigator added 5 commits May 4, 2020 00:05

add exception classes

73e969a

migrate analysis to new exceptions

e07206a

migrate math to new exceptions

1f356f2

migrate synthesis to the new exceptions

a8edfb4

migrate transform to new exceptions

de6e3df

bnavigator force-pushed the slycot-error branch from 683ecd1 to 974155d Compare May 4, 2020 15:12

unify exception call signature

37e45d1

bnavigator force-pushed the slycot-error branch from 974155d to 37e45d1 Compare May 4, 2020 15:18

bnavigator marked this pull request as ready for review May 4, 2020 16:10

repagh reviewed May 4, 2020

View reviewed changes

slycot/analysis.py Outdated Show resolved Hide resolved

slycot/analysis.py Outdated Show resolved Hide resolved

repagh added 2 commits May 5, 2020 16:27

exception messages extracted from docstrings

2f44753

count exception messages in synthesis

89429ad

repagh force-pushed the slycot-error branch from 5100877 to d20d683 Compare May 5, 2020 20:06

Only on indexError should we report unhandled code

0fd2ea5

bnavigator force-pushed the slycot-error branch from d20d683 to cab32f7 Compare May 6, 2020 01:07

bnavigator force-pushed the slycot-error branch from 74a817e to f889af8 Compare May 6, 2020 14:41

bnavigator and others added 7 commits May 6, 2020 16:45

simplify negative info parameter checks

d1b9ab1

actually test error message/info matching. Did reveal some fixes to

dbf643b

exception parsing code

only check docstring if provided

42cfb49

correct sb01bd copy and paste error

b9bfaae

rework exception docstring parser

8b5b8f9

fix python 2 compatibility in regexp

ed027fb

adding a more extensive check of docstring parsing

1877bc3

extend the parser to warnings

d2f684d

bnavigator force-pushed the slycot-error branch from f889af8 to d2f684d Compare May 6, 2020 14:46

update exceptions doc and test [skip ci]

0e8d63c

repagh reviewed May 6, 2020

View reviewed changes

slycot/exceptions.py Outdated Show resolved Hide resolved

slycot/exceptions.py Show resolved Hide resolved

slycot/math.py Outdated Show resolved Hide resolved

slycot/math.py Outdated Show resolved Hide resolved

bnavigator added 3 commits May 7, 2020 03:39

more cleanup, including file attributes for source files

18f054e

dont assertRaisesRegex in Py2

a44c952

repagh reviewed May 7, 2020

View reviewed changes

slycot/math.py Outdated Show resolved Hide resolved

slycot/math.py Outdated Show resolved Hide resolved

slycot/math.py Outdated Show resolved Hide resolved

slycot/exceptions.py Outdated Show resolved Hide resolved

slycot/synthesis.py Outdated Show resolved Hide resolved

bnavigator added 3 commits May 7, 2020 15:05

optional type

d81c7d1

typo in range [skip ci]

ad24193

only parse docstring when iwarn or info

ec8c968

catch all unhandled warnings clean imports

bnavigator added 4 commits May 7, 2020 18:53

bring synthesis.py closer to numpydoc

c0b1069

minor cosmetics in math.py and synthesis.py docstrings [skip ci]

ad28baa

fix infospec detection regex

82b653a

fix iwarn catchall and checkvars initialization

8f60923

bnavigator changed the title ~~Add Slycot exception classes~~ Add Slycot exception and warning classes May 7, 2020

revert file mode changes for py files [skip ci]

91fb7e6

repagh reviewed May 7, 2020

View reviewed changes

slycot/math.py Outdated Show resolved Hide resolved

repagh requested a review from roryyorke May 7, 2020 19:24

comma in mb05nd docstring [skip ci]

fd74855

roryyorke approved these changes May 9, 2020

View reviewed changes

roryyorke mentioned this pull request May 9, 2020

Time to release a new version? #98

Closed

roryyorke merged commit 4ab8e2d into python-control:master May 10, 2020

bnavigator deleted the slycot-error branch December 31, 2020 13:32

Add Slycot exception and warning classes #117

Add Slycot exception and warning classes #117

Uh oh!

Conversation

bnavigator commented May 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

repagh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

repagh commented May 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bnavigator commented May 5, 2020

Uh oh!

repagh commented May 5, 2020

Uh oh!

bnavigator commented May 5, 2020

Uh oh!

bnavigator commented May 6, 2020

Uh oh!

repagh commented May 6, 2020

Uh oh!

bnavigator commented May 6, 2020

Uh oh!

repagh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

repagh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

repagh commented May 7, 2020

Uh oh!

bnavigator commented May 7, 2020

Uh oh!

Uh oh!

roryyorke left a comment

Choose a reason for hiding this comment

Uh oh!

bnavigator commented May 9, 2020

Uh oh!

Uh oh!

bnavigator commented May 3, 2020 •

edited

Loading

repagh commented May 5, 2020 •

edited

Loading