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.

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

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

Closed
mkoeppe opened this issue Mar 5, 2022 · 30 comments
Closed

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

mkoeppe opened this issue Mar 5, 2022 · 30 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 5, 2022

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 mkoeppe added this to the sage-9.6 milestone Mar 5, 2022
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 5, 2022

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 5, 2022

Commit: 557b9d3

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 5, 2022

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

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 5, 2022

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 7, 2022

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

7c1d96fsrc/sage/features/nauty.py: New

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 7, 2022

Changed commit from 557b9d3 to 7c1d96f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 7, 2022

Changed commit from 7c1d96f to 2f1476f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 7, 2022

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

2f1476fGraphGenerators.nauty_geng: Use NautyExecutable(geng).absolute_filename()

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 7, 2022

Changed commit from 2f1476f to ede2c3d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 7, 2022

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

ede2c3dHypergraphGenerators.nauty: Use NautyExecutable(...).absolute_filename()

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 7, 2022

Changed commit from ede2c3d to 9e6892d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 7, 2022

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

9e6892dDiGraphGenerators: Use NautyExecutable(...).absolute_filename()

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 7, 2022

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

27bf2cdGraphGenerators: Use Executable.absolute_filename() in remaining places

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 7, 2022

Changed commit from 9e6892d to 27bf2cd

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 7, 2022

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

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 7, 2022

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 7, 2022

Changed commit from 27bf2cd to ed22266

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 7, 2022

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
Copy link
Contributor

dcoudert commented Mar 8, 2022

comment:13

The patchbot reports pyflakes and deprecation number errors.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 8, 2022

comment:14

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

The pyflakes warnings are not related to the ticket

@dcoudert
Copy link
Contributor

dcoudert commented Mar 8, 2022

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 8, 2022

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 8, 2022

Changed commit from ed22266 to 9107e2c

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 8, 2022

comment:17

Thanks for catching this!

@dcoudert
Copy link
Contributor

dcoudert commented Mar 8, 2022

comment:18

LGTM.

@dcoudert
Copy link
Contributor

dcoudert commented Mar 8, 2022

Reviewer: David Coudert

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 8, 2022

comment:19

Thank you!

@vbraun
Copy link
Member

vbraun commented Mar 12, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants