sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.45k stars 482 forks source link

sage.graphs: Use Executable.absolute_filename() #33465

Closed mkoeppe closed 2 years ago

mkoeppe commented 2 years ago

This is so that the Sage library becomes fully functional even when not being run from within sage-env (which sets PATH).

We add sage.features.nauty to provide features for nauty executables.

To test:

$ venv/bin/python3 -c 'from sage.all import graphs; print(len(list(graphs.fullerenes(60))))'

Depends on #31292 Depends on #31296 Depends on #31590

CC: @dcoudert @dimpase

Component: graph theory

Author: Matthias Koeppe

Branch/Commit: 9107e2c

Reviewer: David Coudert

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

mkoeppe commented 2 years ago

Branch: u/mkoeppe/sage_graphs__use_executable_absolute_filename__

mkoeppe commented 2 years ago

Commit: 557b9d3

mkoeppe commented 2 years ago

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

mkoeppe commented 2 years ago

Description changed:

--- 
+++ 
@@ -1,2 +1,8 @@
-This is so that Sage becomes fully functional even when not being run from within sage-env (which sets PATH).
+This is so that the Sage library becomes fully functional even when not being run from within sage-env (which sets PATH).

+To test:
+
+```
+$ venv/bin/python3 -c 'from sage.all import graphs; print(len(list(graphs.fullerenes(60))))'
+```
+
mkoeppe commented 2 years ago

Last 10 new commits:

d631a03src/sage/features/__init__.py: Fix docstring markup
5f33d4fMerge #31292
88860d3sage.features.Executable.absolute_path: If SAGE_LOCAL != SAGE_VENV, search first in SAGE_VENV/bin, SAGE_LOCAL/bin, then PATH
8801359src/sage/features/__init__.py: Fix import
403bcc1sage.features.Executable: Fix up
9fad95eMerge #31292
a93e9afsrc/sage/features/__init__.py: Simplify Executable.absolute_path
9266709Merge #31292
3566e53Merge #31296
557b9d3GraphGenerators.fullerenes: Use Executable.absolute_filename
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

7c1d96fsrc/sage/features/nauty.py: New
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 557b9d3 to 7c1d96f

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 7c1d96f to 2f1476f

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

2f1476fGraphGenerators.nauty_geng: Use NautyExecutable(geng).absolute_filename()
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 2f1476f to ede2c3d

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

ede2c3dHypergraphGenerators.nauty: Use NautyExecutable(...).absolute_filename()
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from ede2c3d to 9e6892d

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

9e6892dDiGraphGenerators: Use NautyExecutable(...).absolute_filename()
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

27bf2cdGraphGenerators: Use Executable.absolute_filename() in remaining places
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 9e6892d to 27bf2cd

mkoeppe commented 2 years ago

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

mkoeppe commented 2 years ago

Description changed:

--- 
+++ 
@@ -1,4 +1,6 @@
 This is so that the Sage library becomes fully functional even when not being run from within sage-env (which sets PATH).
+
+We add `sage.features.nauty` to provide features for nauty executables.

 To test:
mkoeppe commented 2 years ago

Changed dependencies from #31292, #31296 to #31292, #31296, #31590

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 27bf2cd to ed22266

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

7537643trac #31590: allow passing command to plantri
37a5b40trac #31590: add missing optional statements
ed22266Merge #31590
dcoudert commented 2 years ago
comment:13

The patchbot reports pyflakes and deprecation number errors.

mkoeppe commented 2 years ago
comment:14

The deprecations are coming from the dependencies, specifically #31292.

The pyflakes warnings are not related to the ticket

dcoudert commented 2 years ago
comment:15

There is an error with with benzene due to a missing space in command string

sage -t --warn-long 292.4 --random-seed=170677623432521762806450982723252630968 src/sage/graphs/graph_generators.py
**********************************************************************
File "src/sage/graphs/graph_generators.py", line 1360, in sage.graphs.graph_generators.GraphGenerators.fusenes
Failed example:
    len(list(gen))  # optional benzene
Exception raised:
    Traceback (most recent call last):
      File "/Users/dcoudert/sage/local/var/lib/sage/venv-python3.9/lib/python3.9/site-packages/sage/doctest/forker.py", line 695, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/Users/dcoudert/sage/local/var/lib/sage/venv-python3.9/lib/python3.9/site-packages/sage/doctest/forker.py", line 1093, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.graphs.graph_generators.GraphGenerators.fusenes[6]>", line 1, in <module>
        len(list(gen))  # optional benzene
      File "/Users/dcoudert/sage/local/var/lib/sage/venv-python3.9/lib/python3.9/site-packages/sage/graphs/graph_generators.py", line 1396, in fusenes
        for G in graphs._read_planar_code(sp.stdout):
      File "/Users/dcoudert/sage/local/var/lib/sage/venv-python3.9/lib/python3.9/site-packages/sage/graphs/graph_generators.py", line 1153, in _read_planar_code
        assert header == '>>planar_code<<', 'Not a valid planar code header'
    AssertionError: Not a valid planar code header

It can be fixed this way

diff --git a/src/sage/graphs/graph_generators.py b/src/sage/graphs/graph_generators.py
index e358d07ec6..f047a6ebbe 100644
--- a/src/sage/graphs/graph_generators.py
+++ b/src/sage/graphs/graph_generators.py
@@ -1384,7 +1384,7 @@ class GraphGenerators():

         import shlex
         command = shlex.quote(Benzene().absolute_filename())
-        command += ('b' if benzenoids else '') + ' {0} p'.format(hexagon_count)
+        command += (' b' if benzenoids else '') + ' {0} p'.format(hexagon_count)

         sp = subprocess.Popen(command, shell=True,
                               stdin=subprocess.PIPE, stdout=subprocess.PIPE,

I have tested the ticket with benzene, buckygen, nauty and plantri installed. Other modifications are Ok.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

d0e2179Merge tag '9.6.beta4' into t/33465/sage_graphs__use_executable_absolute_filename__
9107e2csrc/sage/graphs/graph_generators.py: Add missing space in command line
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from ed22266 to 9107e2c

mkoeppe commented 2 years ago
comment:17

Thanks for catching this!

dcoudert commented 2 years ago
comment:18

LGTM.

dcoudert commented 2 years ago

Reviewer: David Coudert

mkoeppe commented 2 years ago
comment:19

Thank you!

vbraun commented 2 years ago

Changed branch from u/mkoeppe/sage_graphs__use_executable_absolute_filename__ to 9107e2c