Permanently get rid of bare except: statements #32788

sagetrac-tmonteil · 2021-10-28T17:37:14Z

A while ago, @jdemeyer used to regularly take care of removing bare except: statements constantly introduced in Sage source code, se e.g. #27427, #14028, #11310, #21687, #24274.

The goal of this ticket (perhaps should it be a task ticket ?) is to take over the work without self-abnegation by:

removing the existing bare except: statements.
ensure that those will not be reintroduced by adding a dedicated plugin to the patchbot (i have code for it but it could only be merged once the exisiting bare except: statements are removed).

CC: @kliem @frederichan-IMJPRG @videlec

Component: misc

Author: Frédéric Chapoton, Jonathan Kliem, Thierry Monteil

Branch/Commit: de4faf6

Reviewer: Jonathan Kliem, Markus Wageringel

Issue created by migration from https://trac.sagemath.org/ticket/32788

The text was updated successfully, but these errors were encountered:

fchapoton · 2021-10-28T18:20:54Z

comment:1

this should rather be done using the pycodestyle linter : code E722

https://pycodestyle.pycqa.org/en/latest/intro.html#error-codes

fchapoton · 2021-10-28T18:23:22Z

comment:2

see #32680 and src/tox.ini

fchapoton · 2021-10-28T18:27:28Z

comment:3

but in fact, I would like that the linters first turn both green before switching on the check for E722,

so, let us hope that #32757 will at last give us green linters..

fchapoton · 2021-10-28T18:28:20Z

comment:4

another aim is to turn on E713 and E714, several recent tickets move on that way : #32766

fchapoton · 2021-10-28T18:59:19Z

comment:5

Current status of E722:

src/sage/schemes/projective/projective_space.py:1696:21: E722 do not use bare 'except'
src/sage/schemes/berkovich/berkovich_space.py:622:13: E722 do not use bare 'except'
src/sage/schemes/product_projective/rational_point.py:498:13: E722 do not use bare 'except'
src/sage/geometry/polyhedron/base.py:5278:17: E722 do not use bare 'except'
src/sage/geometry/polyhedron/library.py:2723:9: E722 do not use bare 'except'
src/sage/graphs/digraph.py:3355:17: E722 do not use bare 'except'
src/sage/graphs/digraph.py:4000:13: E722 do not use bare 'except'
src/sage/graphs/digraph.py:4217:13: E722 do not use bare 'except'
src/sage/rings/qqbar.py:2653:5: E722 do not use bare 'except'
src/sage/interfaces/polymake.py:365:13: E722 do not use bare 'except'
src/sage/interfaces/polymake.py:1596:9: E722 do not use bare 'except'
src/sage/modules/free_module.py:466:9: E722 do not use bare 'except'
src/sage/modules/matrix_morphism.py:1421:9: E722 do not use bare 'except'
src/sage/modules/matrix_morphism.py:1506:9: E722 do not use bare 'except'
src/sage/dynamics/arithmetic_dynamics/berkovich_ds.py:241:17: E722 do not use bare 'except'
src/sage/dynamics/arithmetic_dynamics/berkovich_ds.py:248:17: E722 do not use bare 'except'
src/sage/dynamics/arithmetic_dynamics/berkovich_ds.py:255:13: E722 do not use bare 'except'
src/sage/dynamics/arithmetic_dynamics/berkovich_ds.py:806:13: E722 do not use bare 'except'
src/sage/dynamics/arithmetic_dynamics/berkovich_ds.py:896:25: E722 do not use bare 'except'
src/sage/dynamics/arithmetic_dynamics/berkovich_ds.py:1081:13: E722 do not use bare 'except'

sagetrac-tmonteil · 2021-10-28T19:11:41Z

comment:6

Replying to @fchapoton:

Current status of E722:

src/sage/schemes/projective/projective_space.py:1696:21: E722 do not use bare 'except'
src/sage/schemes/berkovich/berkovich_space.py:622:13: E722 do not use bare 'except'
src/sage/schemes/product_projective/rational_point.py:498:13: E722 do not use bare 'except'
src/sage/geometry/polyhedron/base.py:5278:17: E722 do not use bare 'except'
src/sage/geometry/polyhedron/library.py:2723:9: E722 do not use bare 'except'
src/sage/graphs/digraph.py:3355:17: E722 do not use bare 'except'
src/sage/graphs/digraph.py:4000:13: E722 do not use bare 'except'
src/sage/graphs/digraph.py:4217:13: E722 do not use bare 'except'
src/sage/rings/qqbar.py:2653:5: E722 do not use bare 'except'
src/sage/interfaces/polymake.py:365:13: E722 do not use bare 'except'
src/sage/interfaces/polymake.py:1596:9: E722 do not use bare 'except'
src/sage/modules/free_module.py:466:9: E722 do not use bare 'except'
src/sage/modules/matrix_morphism.py:1421:9: E722 do not use bare 'except'
src/sage/modules/matrix_morphism.py:1506:9: E722 do not use bare 'except'
src/sage/dynamics/arithmetic_dynamics/berkovich_ds.py:241:17: E722 do not use bare 'except'
src/sage/dynamics/arithmetic_dynamics/berkovich_ds.py:248:17: E722 do not use bare 'except'
src/sage/dynamics/arithmetic_dynamics/berkovich_ds.py:255:13: E722 do not use bare 'except'
src/sage/dynamics/arithmetic_dynamics/berkovich_ds.py:806:13: E722 do not use bare 'except'
src/sage/dynamics/arithmetic_dynamics/berkovich_ds.py:896:25: E722 do not use bare 'except'
src/sage/dynamics/arithmetic_dynamics/berkovich_ds.py:1081:13: E722 do not use bare 'except'

A basic grep -R "except:" src/sage returns a few more cases:

src/sage/libs/giac/giac.pyx:        except:
src/sage/libs/giac/giac.pyx:          except:
src/sage/libs/giac/giac.pyx:                except:
src/sage/libs/giac/giac.pyx:        except:
src/sage/libs/giac/giac.pyx:         except:
src/sage/libs/giac/giac.pyx:         except:
src/sage/libs/giac/giac.pyx:               except:
src/sage/libs/giac/giac.pyx:         except:
src/sage/libs/giac/giac.pyx:         except:
src/sage/libs/giac/giac.pyx:         except:
src/sage/libs/giac/giac.pyx:         except:
src/sage/libs/giac/giac.pyx:         except:
src/sage/libs/giac/giac.pyx:         except:
src/sage/libs/giac/giac.pyx:         except:
src/sage/libs/giac/giac.pyx:        except:
src/sage/libs/giac/giac.pyx:     #     except:
src/sage/libs/giac/giac.pyx:            except:
src/sage/libs/giac/giac.pyx:        except:
src/sage/libs/giac/giac.pyx:         except:
src/sage/libs/giac/giac.pyx:#    except:
src/sage/libs/giac/giac.pyx:#    except:
src/sage/libs/giac/giac.pyx:#    except:
src/sage/libs/giac/giac.pyx:#    except:
src/sage/libs/giac/giac.pyx:#    except:
src/sage/libs/giac/giac.pyx:         except:
src/sage/libs/giac/giac.pyx:               except:
src/sage/libs/giac/giac.pyx:        except:
src/sage/graphs/digraph.py:                except:
src/sage/graphs/digraph.py:            except:
src/sage/graphs/digraph.py:            except:
src/sage/graphs/graph_decompositions/tree_decomposition.pyx:        except:
src/sage/interfaces/polymake.py:            except:
src/sage/interfaces/polymake.py:        except:
src/sage/geometry/polyhedron/base.py:                except:
src/sage/geometry/polyhedron/library.py:        except:
src/sage/modules/free_module.py:        except:
src/sage/modules/matrix_morphism.py:        except:
src/sage/modules/matrix_morphism.py:        except:
src/sage/schemes/product_projective/rational_point.py:            except:
src/sage/schemes/berkovich/berkovich_space.py:            except:
src/sage/schemes/projective/projective_space.py:                    except:
src/sage/dynamics/arithmetic_dynamics/berkovich_ds.py:                except:
src/sage/dynamics/arithmetic_dynamics/berkovich_ds.py:                except:
src/sage/dynamics/arithmetic_dynamics/berkovich_ds.py:            except:
src/sage/dynamics/arithmetic_dynamics/berkovich_ds.py:            except:
src/sage/dynamics/arithmetic_dynamics/berkovich_ds.py:                        except:
src/sage/dynamics/arithmetic_dynamics/berkovich_ds.py:            except:
src/sage/dynamics/complex_dynamics/mandel_julia_helper.pyx:    except:
src/sage/rings/polynomial/multi_polynomial_libsingular.pyx:                except:
src/sage/rings/polynomial/multi_polynomial_libsingular.pyx:    except:
src/sage/rings/ring_extension_element.pyx:            except:
src/sage/rings/complex_arb.pyx:        except:
src/sage/rings/qqbar.py:    except:
src/sage/plot/plot3d/shapes.pyx:        except:

fchapoton · 2021-10-28T19:13:42Z

Branch: u/chapoton/32788

fchapoton · 2021-10-28T19:13:42Z

comment:7

pycodestyle probably only looks at py files

New commits:

`db41390`	`fix most E722 (first pass)`

fchapoton · 2021-10-28T19:13:42Z

Commit: db41390

fchapoton · 2021-10-28T19:16:56Z

comment:8

after my branch for py files, there remains

git grep -c "except\s*:" src/
src/doc/en/developer/coding_in_python.rst:1
src/sage/dynamics/complex_dynamics/mandel_julia_helper.pyx:1
src/sage/graphs/graph_decompositions/tree_decomposition.pyx:1
src/sage/libs/giac/giac.pyx:27
src/sage/plot/plot3d/shapes.pyx:1
src/sage/rings/complex_arb.pyx:1
src/sage/rings/polynomial/multi_polynomial_libsingular.pyx:2
src/sage/rings/ring_extension_element.pyx:1

EDIT: the one in the doc is explaining that one should not do that..

sagetrac-git · 2021-10-28T19:25:07Z

Branch pushed to git repo; I updated commit sha1. New commits:

`cffd35e`	`no except: in most pyx files`

sagetrac-git · 2021-10-28T19:25:07Z

Changed commit from db41390 to cffd35e

fchapoton · 2021-10-28T19:25:43Z

comment:10

new commit for pyx files except the giac.pyx mess.

sagetrac-git · 2021-10-28T19:27:25Z

Branch pushed to git repo; I updated commit sha1. New commits:

`279b592`	`fix typo`

sagetrac-git · 2021-10-28T19:27:25Z

Changed commit from cffd35e to 279b592

sagetrac-tmonteil · 2021-10-28T19:30:37Z

comment:12

Replying to @fchapoton:

after my branch for py files, there remains

git grep -c "except\s*:" src/
src/doc/en/developer/coding_in_python.rst:1
src/sage/dynamics/complex_dynamics/mandel_julia_helper.pyx:1
src/sage/graphs/graph_decompositions/tree_decomposition.pyx:1
src/sage/libs/giac/giac.pyx:27
src/sage/plot/plot3d/shapes.pyx:1
src/sage/rings/complex_arb.pyx:1
src/sage/rings/polynomial/multi_polynomial_libsingular.pyx:2
src/sage/rings/ring_extension_element.pyx:1

EDIT: the one in the doc is explaining that one should not do that..

Out of curiosity, did the branch comes from a script ?

sagetrac-tmonteil · 2021-10-28T19:30:57Z

comment:13

After a diagonal view at the commit, the except Exception: in src/sage/interfaces/polymake.py is dubious, and seems to be a consequence of a try:/except: encompassing a pretty long list of tests, which is not recommended as it might mask bugs, see https://www.python.org/dev/peps/pep-0008/#programming-recommendations

fchapoton · 2021-10-28T19:42:03Z

comment:14

Replying to @sagetrac-tmonteil:

Replying to @fchapoton:
after my branch for py files, there remains
git grep -c "except\s*:" src/
src/doc/en/developer/coding_in_python.rst:1
src/sage/dynamics/complex_dynamics/mandel_julia_helper.pyx:1
src/sage/graphs/graph_decompositions/tree_decomposition.pyx:1
src/sage/libs/giac/giac.pyx:27
src/sage/plot/plot3d/shapes.pyx:1
src/sage/rings/complex_arb.pyx:1
src/sage/rings/polynomial/multi_polynomial_libsingular.pyx:2
src/sage/rings/ring_extension_element.pyx:1
EDIT: the one in the doc is explaining that one should not do that..
Out of curiosity, did the branch comes from a script ?

Not a script, just my poor old hands and fatiguated brain..

sagetrac-git · 2021-10-28T19:48:22Z

Changed commit from 279b592 to eb4ae95

sagetrac-git · 2021-10-28T19:48:22Z

Branch pushed to git repo; I updated commit sha1. New commits:

`eb4ae95`	`add import`

sagetrac-tmonteil · 2021-10-28T20:01:14Z

comment:16

Replying to @fchapoton:

Replying to @sagetrac-tmonteil:

Out of curiosity, did the branch comes from a script ?

Not a script, just my poor old hands and fatiguated brain..

Then i should stop developing for Sage given my extremely weak relative productivity in inspecting such issues.

sagetrac-tmonteil · 2021-10-28T20:12:17Z

comment:17

Replying to @fchapoton:

new commit for pyx files except the giac.pyx mess.

I am trying to look at it, it seems that all indentation is shifted by one.

sagetrac-tmonteil · 2021-10-28T21:44:57Z

Changed branch from u/chapoton/32788 to u/tmonteil/32788

sagetrac-tmonteil · 2021-10-28T21:50:38Z

comment:19

I added 2 commits dealing with giac.pyx to your branch. The first commit only fixes bad indentation that made my editor crazy, the second one fixes most try/except statements (most of them seem just useless).

The last two remaining cases are related with some pari_lock and need to be explored.

New commits:

`bcdb620`	`#32788 : fix intentations for libs/giac/giac.pyx`
`da3e437`	`#32788 : remove useless try/except statements in src/sage/libs/giac/giac.pyx`

sagetrac-tmonteil · 2021-10-28T21:50:38Z

Changed commit from eb4ae95 to da3e437

sagetrac-tmonteil · 2021-10-28T22:19:16Z

comment:20

fchapoton · 2021-10-29T11:44:15Z

Changed commit from da3e437 to be872c1

fchapoton · 2021-11-18T12:40:06Z

comment:59

green bot again

fchapoton · 2021-11-21T19:50:36Z

comment:60

can we please move on here ?

sagetrac-tmonteil · 2021-11-29T19:06:27Z

Changed branch from u/chapoton/except_fix_branch to u/tmonteil/except_fix_branch

sagetrac-tmonteil · 2021-11-29T19:10:54Z

Changed commit from ec236ec to 9522357

sagetrac-tmonteil · 2021-11-29T19:10:54Z

comment:62

I cherry-picked the relevant commits and merged with 9.5.beta6 so that git diff trac/u/chapoton/except_fix_branch trac/u/tmonteil/except_fix_branch leads to:

--- a/src/sage/interfaces/polymake.py
+++ b/src/sage/interfaces/polymake.py
@@ -364,7 +364,7 @@ class PolymakeAbstract(ExtraTabCompletion, Interface):
             try:
                 x = RDF(x)
                 return '{}'.format(x)
-            except Exception:
+            except (TypeError, ValueError):
                 pass
 
             raise NotImplementedError

I am not sure if i have to revert that change.

Last 10 new commits:

`be872c1`	`some fixes in giac, schemes, geometry`
`35cab30`	`Merge branch 'u/chapoton/32788' of git://trac.sagemath.org/sage into u/gh-kliem/32788`
`0113b72`	`code style again`
`6c08ce1`	`remove blank except in GIAC call`
`04a1f01`	`fix distinction between two calls`
`9905930`	`remove code duplication`
`d3df701`	`#32788 : ensure bare except: statements will not reappear`
`e92b905`	`mark polymake_expect import as optional`
`f463e49`	`comment before`
`9522357`	`Merge branch 'develop.9.5.beta6' into HEAD`

mkoeppe · 2021-12-18T19:53:12Z

comment:63

Stalled in needs_review or needs_info; likely won't make it into Sage 9.5.

fchapoton · 2021-12-28T16:26:59Z

comment:64

red branch => needs work

fchapoton · 2022-01-14T19:37:51Z

Changed branch from u/tmonteil/except_fix_branch to public/except_fix_branch

fchapoton · 2022-01-14T19:37:51Z

Changed commit from 9522357 to db57c4f

fchapoton · 2022-01-14T19:37:51Z

comment:65

rebased on 9.5.rc1

New commits:

`58865ce`	`various lgtm-suggested details`
`db57c4f`	`Merge branch 'u/tmonteil/except_fix_branch' in 9.5.rc1`

sagetrac-git · 2022-01-14T19:48:48Z

Changed commit from db57c4f to de4faf6

sagetrac-git · 2022-01-14T19:48:48Z

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

`de4faf6`	`Merge branch 'u/tmonteil/except_fix_branch' in 9.5.rc1`

fchapoton · 2022-01-14T19:49:24Z

comment:67

now correctly rebased, needs test by somebody having polymake

mwageringel · 2022-02-01T19:02:49Z

comment:68

There are two polymake test failures (see below), but they are not related to this ticket, so are best dealt with in another ticket.

I am ready to set this ticket to positive, if someone could confirm that the new pycodestyle check still passes with 9.5 merged. Unfortunately, I have difficulties in running tox on my machine.

By the way, this ticket will also fix a random doctest failure in coding/linear_code.py which is due to an AlarmInterrupt being swallowed by a bare except.

sage -t --long --warn-long 69.2 --random-seed=17721019333867817533235853689613643341 src/doc/en/thematic_tutorials/geometry/polyhedra_tutorial.rst
**********************************************************************
File "src/doc/en/thematic_tutorials/geometry/polyhedra_tutorial.rst", line 650, in doc.en.thematic_tutorials.geometry.polyhedra_tutorial
Failed example:
    Polyhedron(vertices=V, backend='polymake')               # optional - polymake
Expected:
    A 3-dimensional polyhedron in (Number Field in sqrt5 with defining polynomial x^2 - 5)^3 defined as the convex hull of 20 vertices
Got:
    A 3-dimensional polyhedron in (Number Field in sqrt5 with defining polynomial x^2 - 5 with sqrt5 = 2.236067977499790?)^3 defined as the convex hull of 20 v
ertices

sage -t --long --warn-long 69.2 --random-seed=201256750698610760769760302766889916051 src/sage/interfaces/polymake.py
**********************************************************************
File "src/sage/interfaces/polymake.py", line 2189, in sage.interfaces.polymake.PolymakeExpect._eval_line
Failed example:
    c.N_VERTICES                      # optional - polymake_expect
Expected:
    32768
Got:
    Can't locate object method "description" via package "32768" (perhaps you forgot to load "32768"?) at input line 1.

mwageringel · 2022-02-01T19:02:49Z

Changed reviewer from Jonathan Kliem to Jonathan Kliem, Markus Wageringel

mwageringel · 2022-02-01T19:23:04Z

comment:69

Never mind. I have tried it on a different machine and pycodestyle-minimal passes, so I think this is good to go.

slel · 2022-02-01T20:33:17Z

comment:70

In src/sage/libs/giac/giac.pyx, docstring
indentation for loadgiacgen seems off.

mwageringel · 2022-02-02T19:10:27Z

comment:71

Replying to @slel:

In src/sage/libs/giac/giac.pyx, docstring
indentation for loadgiacgen seems off.

I am happy to review it if anyone wants to change that, but I do not think it is necessary here.

vbraun · 2022-02-12T22:05:36Z

Changed branch from public/except_fix_branch to de4faf6

sagetrac-tmonteil mannequin added this to the sage-9.5 milestone Oct 28, 2021

sagetrac-tmonteil mannequin added p: major / 3 labels Oct 28, 2021

mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021

fchapoton added s: needs work and removed s: needs review labels Dec 28, 2021

fchapoton added s: needs review and removed s: needs work labels Jan 14, 2022

mwageringel added s: positive review and removed s: needs review labels Feb 1, 2022

vbraun removed the s: positive review label Feb 12, 2022

vbraun closed this as completed in 3834fde Feb 12, 2022

kliem mentioned this issue Feb 16, 2022

Move construction methods to Polyhedron_base5 #33123

Closed

Permanently get rid of bare except: statements #32788

Permanently get rid of bare except: statements #32788

Comments

sagetrac-tmonteil mannequin commented Oct 28, 2021

fchapoton commented Oct 28, 2021

fchapoton commented Oct 28, 2021

fchapoton commented Oct 28, 2021

fchapoton commented Oct 28, 2021

fchapoton commented Oct 28, 2021

sagetrac-tmonteil mannequin commented Oct 28, 2021

fchapoton commented Oct 28, 2021

fchapoton commented Oct 28, 2021

fchapoton commented Oct 28, 2021

fchapoton commented Oct 28, 2021

sagetrac-git mannequin commented Oct 28, 2021

sagetrac-git mannequin commented Oct 28, 2021

fchapoton commented Oct 28, 2021

sagetrac-git mannequin commented Oct 28, 2021

sagetrac-git mannequin commented Oct 28, 2021

sagetrac-tmonteil mannequin commented Oct 28, 2021

sagetrac-tmonteil mannequin commented Oct 28, 2021

fchapoton commented Oct 28, 2021

sagetrac-git mannequin commented Oct 28, 2021

sagetrac-git mannequin commented Oct 28, 2021

sagetrac-tmonteil mannequin commented Oct 28, 2021

sagetrac-tmonteil mannequin commented Oct 28, 2021

sagetrac-tmonteil mannequin commented Oct 28, 2021

sagetrac-tmonteil mannequin commented Oct 28, 2021

sagetrac-tmonteil mannequin commented Oct 28, 2021

sagetrac-tmonteil mannequin commented Oct 28, 2021

fchapoton commented Oct 29, 2021

fchapoton commented Nov 18, 2021

fchapoton commented Nov 21, 2021

sagetrac-tmonteil mannequin commented Nov 29, 2021

sagetrac-tmonteil mannequin commented Nov 29, 2021

sagetrac-tmonteil mannequin commented Nov 29, 2021

mkoeppe commented Dec 18, 2021

fchapoton commented Dec 28, 2021

fchapoton commented Jan 14, 2022

fchapoton commented Jan 14, 2022

fchapoton commented Jan 14, 2022

sagetrac-git mannequin commented Jan 14, 2022

sagetrac-git mannequin commented Jan 14, 2022

fchapoton commented Jan 14, 2022

mwageringel commented Feb 1, 2022

mwageringel commented Feb 1, 2022

mwageringel commented Feb 1, 2022

slel commented Feb 1, 2022

mwageringel commented Feb 2, 2022

vbraun commented Feb 12, 2022