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.

Sign up for GitHub

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

Jump to bottom

Replace use of sage.misc.package.PackageNotFoundError, is_package_installed by features #30607

Closed
mkoeppe opened this issue Sep 18, 2020 · 21 comments
Closed

Replace use of sage.misc.package.PackageNotFoundError, is_package_installed by features #30607

mkoeppe opened this issue Sep 18, 2020 · 21 comments
Labels
c: build: configure p: major / 3 t: enhancement
Milestone
sage-9.2

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Sep 18, 2020

In this ticket and #30616, we get rid of the remaining uses of PackageNotFoundError for dealing with optional extensions etc. by using sage.features instead.

$ git grep sage.misc.package
src/sage/databases/cremona.py:                         from sage.misc.package import is_package_installed
src/sage/databases/jones.py:                           from sage.misc.package import PackageNotFoundError
src/sage/game_theory/normal_form_game.py:              from sage.misc.package import PackageNotFoundError
src/sage/game_theory/normal_form_game.py:              from sage.misc.package import PackageNotFoundError
src/sage/graphs/graph.py:                              from sage.misc.package import PackageNotFoundError
src/sage/graphs/graph.py:                              from sage.misc.package import PackageNotFoundError
src/sage/graphs/graph.py:                              from sage.misc.package import PackageNotFoundError
src/sage/groups/braid.py:                              from sage.misc.package import PackageNotFoundError
src/sage/matrix/matrix_space.py:                       from sage.misc.package import PackageNotFoundError
src/sage/sat/solvers/cryptominisat.py:                 from sage.misc.package import PackageNotFoundError
src/sage/sat/solvers/picosat.py:                       from sage.misc.package import PackageNotFoundError

After this ticket and #30616, only uses for optional packages that do not exist any more remain.

src/sage/interfaces/kash.py:                           from sage.misc.package import PackageNotFoundError
src/sage/rings/polynomial/multi_polynomial_ideal.py:   from sage.misc.package import PackageNotFoundError

(see #30617 for ginv, #25488 for kash)

CC: @kiwifb @seblabbe @jhpalmieri

Component: build: configure

Author: Matthias Koeppe

Branch/Commit: ec66d89

Reviewer: Sébastien Labbé

Issue created by migration from https://trac.sagemath.org/ticket/30607
@mkoeppe mkoeppe added this to the sage-9.2 milestone Sep 18, 2020
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 19, 2020

Branch: u/mkoeppe/replace_use_of__sage_misc_package_packagenotfounderror___is_package_installed_by_features

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 19, 2020

Commit: 48f663b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 19, 2020

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

48f663bsage.features.databases.DatabaseJones: New, use it in sage.databases.jones

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 19, 2020

Author: Matthias Koeppe, ...

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 19, 2020

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

795a18bFeatureNotPresentError: Take resolution from feature if not passed
3a1212cNormalFormGame._solve_lrs: Use FeatureNotPresentError instead of PackageNotFoundError

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 19, 2020

Changed commit from 48f663b to 3a1212c

@mkoeppe

This comment has been minimized.

Sign in to view
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 20, 2020

Changed author from Matthias Koeppe, ... to Matthias Koeppe

@mkoeppe

This comment has been minimized.

Sign in to view
@mkoeppe

This comment has been minimized.

Sign in to view
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 23, 2020

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

9c5154bMerge tag '9.2.beta13' into t/30607/replace_use_of__sage_misc_package_packagenotfounderror___is_package_installed_by_features
4894a8cPackageNotFoundError: Add documentation/tests
ec66d89src/sage/features/databases.py: Add doctest for 100% coverage

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 23, 2020

Changed commit from 3a1212c to ec66d89

@seblabbe
Copy link
Contributor

comment:10

Without the Jones database installed, I confirm the following works:

sage: from sage.features.databases import DatabaseJones                         
sage: DatabaseJones().is_present()                                              
FeatureTestResult("John Jones's tables of number fields", False)
sage: bool(_)                                                                   
False
sage: DatabaseJones().absolute_path()                                           
Traceback (most recent call last):
...
FeatureNotPresentError: John Jones's tables of number fields is not available.
'jones/jones.sobj' not found in any of ['/home/slabbe/GitBox/sage/local/share']
To install John Jones's tables of number fields you can try to run 'sage -i database_jones_numfield'.

After doing sage -i database_jones_numfield, I confirm the following works:

sage: from sage.features.databases import DatabaseJones                         
sage: DatabaseJones().is_present()                                              
FeatureTestResult("John Jones's tables of number fields", True)
sage: DatabaseJones().absolute_path()                                           
'/home/slabbe/GitBox/sage/local/share/jones/jones.sobj'

@seblabbe
Copy link
Contributor

comment:11

Without lrslib installed, I confirm the following works (error is properly raised):

sage: p1 = matrix([[-7, -5, 5],
....:              [5, 5, 3],
....:              [1, -6, 1]])
sage: p2 = matrix([[-9, 7, 9],
....:              [6, -2, -3],
....:              [-4, 6, -10]])
sage: biggame = NormalFormGame([p1, p2])
sage: biggame._solve_lrs()
Traceback (most recent call last):
...
FileNotFoundError: [Errno 2] No such file or directory: 'lrsnash'
...
During handling of the above exception, another exception occurred:
...
FeatureNotPresentError: lrslib is not available.
Calling lrsnash failed with [Errno 2] No such file or directory: 'lrsnash'
To install lrslib you can try to run 'sage -i lrslib'.
Further installation instructions might be available at http://cgm.cs.mcgill.ca/~avis/C/lrs.html.

And then, I confirm this works as suggested by the error message:

sage: !sage -i lrslib  
...
sage: biggame._solve_lrs()                                                      
[[(0, 1, 0), (1, 0, 0)],
 [(1/3, 2/3, 0), (0, 1/6, 5/6)],
 [(1/3, 2/3, 0), (1/7, 0, 6/7)],
 [(1, 0, 0), (0, 0, 1)]]

@seblabbe
Copy link
Contributor

comment:12

Finally, I confirm the new behavior for PackageNotFoundError:

sage: from sage.misc.package import PackageNotFoundError                        
sage: raise PackageNotFoundError("my_package")                                  
...
DeprecationWarning: Instead of raising PackageNotFoundError, raise sage.features.FeatureNotPresentError
See https://trac.sagemath.org/30607 for details.
...
Traceback (most recent call last):
...
PackageNotFoundError: the package 'my_package' was not found. You can install it by running 'sage -i my_package' in a shell

@seblabbe
Copy link
Contributor

Reviewer: Sébastien Labbé

@seblabbe
Copy link
Contributor

comment:13

sage -tp --optional=sage,optional,external src/sage/databases/jones.py src/sage/features/__init__.py src/sage/features/databases.py src/sage/game_theory/normal_form_game.py src/sage/misc/package.py --show-skipped works ok (except one issue already fixed in #30632).

To me it is a positive review. Let's wait for the patchbot to see if everything is green.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 26, 2020

comment:14

I think the pyflakes warning is a flake

@seblabbe
Copy link
Contributor

seblabbe commented Oct 2, 2020

comment:17

Sorry for the delay in follow-up, I was busy with other TODOs this week.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 3, 2020

comment:18

Thanks!

@vbraun
Copy link
Member

vbraun commented Oct 5, 2020

Changed branch from u/mkoeppe/replace_use_of__sage_misc_package_packagenotfounderror___is_package_installed_by_features to ec66d89

@vbraun vbraun closed this as completed in ef0ab12 Oct 5, 2020
mkoeppe added a commit to mkoeppe/sage that referenced this issue Apr 23, 2024
@mkoeppe
src/sage/misc/package.py: Remove code deprecated in sagemath#30607 (2…
a91c5c9 
…020)
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
a53ed51 
sagemathgh-37855: `sage.misc.package`: Remove deprecated code
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Remove code deprecated in:
- sagemath#31013 (2022)
- sagemath#30747 (2020)
- sagemath#30607 (2020)

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37855
Reported by: Matthias Köppe
Reviewer(s): François Bissey, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this issue May 2, 2024
6753fbd 
sagemathgh-37855: `sage.misc.package`: Remove deprecated code
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Remove code deprecated in:
- sagemath#31013 (2022)
- sagemath#30747 (2020)
- sagemath#30607 (2020)

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37855
Reported by: Matthias Köppe
Reviewer(s): François Bissey, Matthias Köppe
@mantepse mantepse mentioned this issue May 10, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c: build: configure p: major / 3 t: enhancement
Projects
None yet
Milestone
sage-9.2
Development

No branches or pull requests
3 participants
@vbraun @seblabbe @mkoeppe