sage.features.Executable: Add method absolute_filename

[mkoeppe]

We refactor Executable and StaticFile through a base class FileFeature.

The method absolute_filename replaces StaticFile.absolute_path and is now also available for Executable. (This is for #33440/#31296/#30818.)

The renaming from StaticFile.absolute_path to absolute_filename (with deprecation) is preparation for reclaiming the name absolute_path for a method that returns a Path instead of a str.

We also add some type annotations.

Follow-up: #31296

CC: @tobiasdiez @fchapoton @seblabbe @orlitzky @kiwifb @antonio-rojas @tornaria

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: e9568e9

Reviewer: Tobias Diez

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

[mkoeppe]

comment:1

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

[mkoeppe]

Author: Matthias Koeppe

[mkoeppe]

Replying to @mkoeppe:

In particular, we change StaticFile.absolute_path to return a Path.

Actually this would lead to errors like

File "src/sage/databases/cremona.py", line 1552, in sage.databases.cremona.LargeCremonaDatabase.degphi
Failed example:
    c = CremonaDatabase()
Exception raised:
...
      File "/Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/local/var/lib/sage/venv-python3.9/lib/python3.9/site-packages/sage/databases/sql_db.py", line 1066, in __init__
        elif (filename[-3:] != '.db'):
    TypeError: 'PosixPath' object is not subscriptable

[mkoeppe]

Branch: u/mkoeppe/sage_features_staticfile__executable__use_pathlib__add_type_annotations

[mkoeppe]

Commit: 32ce831

[mkoeppe]

New commits:

32ce831

sage.features.Executable.absolute_path: New

[sagetrac-git]

Changed commit from 32ce831 to 88860d3

[sagetrac-git]

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

88860d3

sage.features.Executable.absolute_path: If SAGE_LOCAL != SAGE_VENV, search first in SAGE_VENV/bin, SAGE_LOCAL/bin, then PATH

[sagetrac-git]

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

[sagetrac-git]

Changed commit from 88860d3 to 32ce831

[sagetrac-git]

Changed commit from 32ce831 to 8bb7e3a

[sagetrac-git]

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

8bb7e3a

src/sage/features/__init__.py: Relax doctest

[sagetrac-git]

Changed commit from 57531d0 to 66b8125

[sagetrac-git]

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

66b8125

sage.features.FileFeature: Add doc

[mkoeppe]

comment:38

Replying to @tobiasdiez:

Please replace this by a meaningful doctest that actually shows how to use the class.

Done now

[sagetrac-git]

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

d631a03

src/sage/features/__init__.py: Fix docstring markup

[sagetrac-git]

Changed commit from 66b8125 to d631a03

[tobiasdiez]

comment:40

Thanks for the follow-up!

One last comment, otherwise it looks good to me:
The docs for absolute_filename in FileFeature now still refers to "executable", which should be replaced by "feature".

[tobiasdiez]

comment:41

By the way, do the inheriting classes (Executable and StaticFile) really need to provide docstrings? Sphinx should copy them from the base class or not?

[sagetrac-git]

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

e9568e9

FileFeature.absolute_filename: Fix docstring

[sagetrac-git]

Changed commit from d631a03 to e9568e9

[mkoeppe]

comment:43

Replying to @tobiasdiez:

By the way, do the inheriting classes (Executable and StaticFile) really need to provide docstrings?

Yes, per our style guide.

[tobiasdiez]

comment:45

Replying to @mkoeppe:

Replying to @tobiasdiez:

By the way, do the inheriting classes (Executable and StaticFile) really need to provide docstrings?

Yes, per our style guide.

What is the reason behind this requirement/convention?

[tobiasdiez]

Reviewer: Tobias Diez

[mkoeppe]

comment:47

Thanks for reviewing!

[mkoeppe]

comment:48

Replying to @tobiasdiez:

What is the reason behind this requirement/convention?

It was already there when I started Sage dev. It's a bit annoying at times and leads to a lot of copy+paste, but it does force people to write doctests. And having the policy scales better than renegotiating the style with every new developer.

[vbraun]

Changed branch from u/mkoeppe/sage_features_staticfile__executable__use_pathlib__add_type_annotations to e9568e9