Replace use of `sage.misc.package.PackageNotFoundError` (2); lazy_import: Add keyword argument 'feature'

Question

Replace use of `sage.misc.package.PackageNotFoundError` (2); lazy_import: Add keyword argument 'feature'

mkoeppe opened this issue 5 years ago · 19 comments

mkoeppe commented 5 years ago

This helps with the modularization of sagelib.

We demonstrate its use by simplifying:

sage.groups.braid (which depends on the optional library libbraiding via sage.libs.libbraiding). Actually a standard package
sage.sat.solvers.cryptominisat, .picosat
sage.matrix.matrix_space (meataxe)
sage.graphs.graph (tdlib, mcqd)

This simplification helps eliminate the use of sage.misc.package.PackageNotFoundError. See #30607.

CC: @kiwifb @jhpalmieri @slel @seblabbe @dcoudert

Component: refactoring

Author: Matthias Koeppe

Branch: e1f6624

Reviewer: David Coudert

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

Answer 1 · 2020-09-20T00:50:07.000Z

Branch: u/mkoeppe/lazy_import__add_keyword_argument__feature_

Answer 2 · 2020-09-20T01:09:01.000Z

Commit: 30613c8

Answer 3 · 2020-09-20T01:09:01.000Z

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

`fdc62f7`	`src/sage/sat/solvers/cryptominisat.py: Use lazy_import with feature instead of try/except`
`30613c8`	`src/sage/sat/solvers/picosat.py: Use lazy_import with feature instead of try/except`

Answer 4 · 2020-09-20T01:26:03.000Z

Description changed:

--- 
+++ 
@@ -1,5 +1,7 @@
 This helps with the modularization of sagelib.
 
-We demonstrate its use by simplifying `sage.groups.braid` (which depends on the optional library `libbraiding` via `sage.libs.libbraiding`).
+We demonstrate its use by simplifying:
+- `sage.groups.braid` (which depends on the optional library `libbraiding` via `sage.libs.libbraiding`). Actually a standard package
+- `sage.sat.solvers.cryptominisat`, `.picosat`
+- `sage.matrix.matrix_space` (`meataxe`)
 
-

Answer 5 · 2020-09-20T01:26:03.000Z

Author: Matthias Koeppe

Answer 6 · 2020-09-21T02:07:09.000Z

Description changed:

--- 
+++ 
@@ -4,4 +4,10 @@
 - `sage.groups.braid` (which depends on the optional library `libbraiding` via `sage.libs.libbraiding`). Actually a standard package
 - `sage.sat.solvers.cryptominisat`, `.picosat`
 - `sage.matrix.matrix_space` (`meataxe`)
+- `sage.graphs.graph` (`tdlib`, `mcqd`)
 
+This simplification helps eliminate the use of `sage.misc.package.PackageNotFoundError`.  See #30607.
+
+
+
+

Answer 7 · 2020-09-21T02:32:18.000Z

Changed commit from 30613c8 to e1f6624

Answer 8 · 2020-09-21T02:32:18.000Z

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

`f2b5a1c`	`Graph.treewidth: Fix up logic in algorithm selection`
`e1f6624`	`Graph.{treewidth,clique_maximum,clique_number}: Use features instead of sage.misc.package.PackageNotFoundError`

Answer 9 · 2020-09-23T14:18:36.000Z

comment:8

I tried before and after installing tdlib, mcqd, cryptominisat, pycosat (not obvious to know that its picosat), meataxe. Note that libbraiding was already installed (is it standard now ?). It's working well.

However, we have several pyflakes errors (see patchbot)

src/sage/graphs/graph.py:6419:20 undefined name 'mcqd'
src/sage/graphs/graph.py:6517:24 undefined name 'mcqd'

src/sage/groups/braid.py:82:1 'sage.misc.package.PackageNotFoundError' imported but unused
src/sage/groups/braid.py:1080:13 undefined name 'rightnormalform'
src/sage/groups/braid.py:1096:13 undefined name 'centralizer'
src/sage/groups/braid.py:1115:13 undefined name 'supersummitset'
src/sage/groups/braid.py:1138:13 undefined name 'greatestcommondivisor'
src/sage/groups/braid.py:1158:13 undefined name 'leastcommonmultiple'
src/sage/groups/braid.py:1183:13 undefined name 'conjugatingbraid'
src/sage/groups/braid.py:1208:13 undefined name 'conjugatingbraid'
src/sage/groups/braid.py:1236:15 undefined name 'ultrasummitset'
src/sage/groups/braid.py:1261:16 undefined name 'thurston_type'
src/sage/groups/braid.py:1326:24 undefined name 'rigidity'
src/sage/groups/braid.py:1358:15 undefined name 'sliding_circuits'

src/sage/matrix/matrix_space.py:287:24 undefined name 'Matrix_gfpn_dense'

src/sage/sat/solvers/cryptominisat.py:72:24 undefined name 'Solver'

src/sage/sat/solvers/picosat.py:63:23 undefined name 'solve'

Answer 10 · 2020-09-23T14:22:52.000Z

comment:9

Replying to @dcoudert:

Note that libbraiding was already installed (is it standard now ?).

Yes, see build/pkgs/libbraiding/type which you can either
check with this command:

$ cat build/pkgs/libbraiding/type
standard

or by viewing that file online:

https://github.com/sagemath/sage-prod/blob/develop/build/pkgs/libbraiding/type

This happened in

#25705 Make libhomfly and libbraiding standard packages

which was merged in Sage 8.4.beta0.

Answer 11 · 2020-09-23T14:53:18.000Z

comment:10

Thanks.

Can we do something for the pyflakes issues or is it the prize to pay for lazy import ?

Answer 12 · 2020-09-23T17:05:16.000Z

comment:11

Unfortunately that's the prize to pay...

Answer 13 · 2020-09-23T17:37:29.000Z

Reviewer: David Coudert

Answer 14 · 2020-09-23T17:37:29.000Z

comment:12

So then.

Answer 15 · 2020-09-23T17:51:26.000Z

comment:13

Thank you!

Answer 16 · 2020-09-23T18:11:07.000Z

comment:14

Replying to @mkoeppe:

Unfortunately that's the price to pay...

Follow-up: #30647 - Make lazy_import more friendly to pyflakes and other static checkers

Answer 17 · 2020-10-05T20:13:08.000Z

Changed branch from u/mkoeppe/lazy_import__add_keyword_argument__feature_ to e1f6624

Answer 18 · 2020-12-10T08:38:09.000Z

comment:16

I'm going to add the missing documentation on #30587.

Answer 19 · 2020-12-10T08:38:09.000Z

Changed commit from e1f6624 to none