Immutable graphs must not be relabeled

[nathanncohen]

Now

sage: Graph(graphs.PetersenGraph(), immutable=True).relabel({})
...
ValueError: Thou shalt not relabel an immutable graph

Before, the following happened

sage: Graph(graphs.PetersenGraph(), data_structure="static_sparse").relabel({})
...
AttributeError: 'StaticSparseBackend' object has no attribute 'vertex_ints'

This is actually another bug, as there should be a vertex_ints variable in static sparse graphs. But that's another problem to be addressed in another ticket :-P

Nathann

Depends on #15619

CC: @simon-king-jena

Component: graph theory

Author: Nathann Cohen

Branch/Commit: u/ncohen/15622 @ 6398780

Reviewer: Simon King

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

[nathanncohen]

Branch: u/ncohen/15622

[nathanncohen]

Changed dependencies from #15603 to #15619

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

0f57805	trac #15622: Immutable graphs must not be relabeled
c441178	trac #15619: bug in the former definition; exception to avoid it in the future
0229348	trac #15619: Pickling of immutable graphs
7860f39	trac #15603: "immutable=True" for Graph/Digraph `__init__` and copy()
2525d22	Trac 15278: Fix syntax error in doc test
fcf9e30	Trac 15278: Only graphs that use the static backend are identical with their copy
8fc9c94	Merge branch 'develop' into ticket/15278
51d6328	Trac 15278: Error messages explain how to create (im)mutable graph copy
2fc8a77	Make digraphs using the static backend immutable and hashable
07bad46	Merge branch 'u/ncohen/15491' of git://trac.sagemath.org/sage into ticket/15278

[sagetrac-git]

Commit: 0f57805

[simon-king-jena]

comment:4

The code looks good. Perhaps the error message should hint at how to create a mutable copy?

[nathanncohen]

comment:5

OKayyyyyyyy.. What about this ?

Nathann

[sagetrac-git]

Changed commit from 0f57805 to 529f785

[sagetrac-git]

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

529f785

trac #15622: More informative exception message

[simon-king-jena]

comment:7

Oops:

sage -t --long src/sage/graphs/base/static_sparse_backend.pyx
**********************************************************************
File "src/sage/graphs/base/static_sparse_backend.pyx", line 422, in sage.graphs.base.static_sparse_backend.StaticSparseBackend.relabel
Failed example:
    g.relabel([],True)
Expected:
    Traceback (most recent call last):
    ...
    ValueError: Thou shalt not remove a vertex from an immutable graph
Got:
    <BLANKLINE>
    Traceback (most recent call last):
      File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 480, in _run
        self.execute(example, compiled, test.globs)
      File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 839, in execute
        exec compiled in globs
      File "<doctest sage.graphs.base.static_sparse_backend.StaticSparseBackend.relabel[2]>", line 1, in <module>
        g.relabel([],True)
    AttributeError: 'sage.graphs.base.static_sparse_backend.StaticSpars' object has no attribute 'relabel'

That is the only error, and I'll probably fix it in a review commit.

[simon-king-jena]

comment:8

WTF??????

When I do the obvious and change StaticSparseCGraph into StaticSparseBackend, I get

Failed example:
    g.relabel([],True)
Expected:
    Traceback (most recent call last):
    ...
    ValueError: Thou shalt not remove a vertex from an immutable graph
Got:
    <BLANKLINE>
    Traceback (most recent call last):
      File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 480, in _run
        self.execute(example, compiled, test.globs)
      File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 839, in execute
        exec compiled in globs
      File "<doctest sage.graphs.base.static_sparse_backend.StaticSparseBackend.relabel[2]>", line 1, in <module>
        g.relabel([],True)
      File "static_sparse_backend.pyx", line 428, in sage.graphs.base.static_sparse_backend.StaticSparseBackend.relabel (sage/graphs/base/static_sparse_backend.c:4740)
    ValueError: Thou shalt not relabel an immutable graph

Where does the blankline come from? And why is there no blankline when doing the test interactively?

[simon-king-jena]

comment:9

Ahaaa! You had a wrong class and a wrong error message in your test ("remove a vertex" versus "relabel").

[nathanncohen]

comment:10

I don't get what the "obvious change from StaticSparseCGraph into StaticSparseBackend" could be O_o

Nathann

[simon-king-jena]

comment:11

Replying to @nathanncohen:

I don't get what the "obvious change from StaticSparseCGraph into StaticSparseBackend" could be O_o

diff --git a/src/sage/graphs/base/static_sparse_backend.pyx b/src/sage/graphs/base/static_sparse_backend.pyx
index 9b5a8fe..e5b889e 100644
--- a/src/sage/graphs/base/static_sparse_backend.pyx
+++ b/src/sage/graphs/base/static_sparse_backend.pyx
@@ -417,12 +417,12 @@ class StaticSparseBackend(CGraphBackend):
 
         TEST::
 
-            sage: from sage.graphs.base.static_sparse_backend import StaticSparseCGraph
-            sage: g = StaticSparseCGraph(graphs.PetersenGraph())
+            sage: from sage.graphs.base.static_sparse_backend import StaticSparseBackend
+            sage: g = StaticSparseBackend(graphs.PetersenGraph())
             sage: g.relabel([],True)
             Traceback (most recent call last):
             ...
-            ValueError: Thou shalt not remove a vertex from an immutable graph
+            ValueError: Thou shalt not relabel an immutable graph
 
         """
         raise ValueError("Thou shalt not relabel an immutable graph")

StaticSparceCGraph simply is the wrong class.

[simon-king-jena]

comment:12

For the record: It already takes 5 minutes to push my review commit.

[simon-king-jena]

Changed branch from u/ncohen/15622 to u/SimonKing/ticket/15622

[nathanncohen]

comment:14

Oh. Ahem. Right O_O;;;;;;

Sorryyyyyyyyyyyyyyyyyyyyyyyyyyy O_O;;;;;

Nathann

Review commit : I often Ctrl+C it before it ends. Trac sends the mails almost instantaneously, the trac ticket is updated at the same time. I just have no idea what it does afterwards.

[nathanncohen]

Changed branch from u/SimonKing/ticket/15622 to u/ncohen/15622

[simon-king-jena]

comment:15

It seems that once more I have to manually upload the commit.

[simon-king-jena]

Changed branch from u/ncohen/15622 to u/SimonKing/ticket/15622

[simon-king-jena]

Changed commit from 529f785 to 2245c02

[nathanncohen]

comment:16

I think it's because you should set the branch name before pushing the commit. Otherwise nothing is triggered on the trac server's side.

Nathann

[nathanncohen]

comment:17

Okay. We are now in the Volker-Feared situation of a ticket which has a positive review, with a dependency which is still in needs_review :-P

Thanks for this commit !

Nathann

[nathanncohen]

comment:18

By the way, isn't the trac server 99999x times faster than yesterday ? Andrew said he would do something about it and it looks like he did O_O

Nathann

[vbraun]

Reviewer: Simon King

[vbraun]

comment:22

Various doctests fail (see also patchbot)

[nathanncohen]

comment:23

Gloops. Right.

[nathanncohen]

Changed branch from u/SimonKing/ticket/15622 to u/ncohen/15622

[nathanncohen]

Changed commit from 2245c02 to 529f785

[nathanncohen]

comment:24

That's the only way I found to make this work before #15623 is merged T_T

Nathann

[nathanncohen]

Changed commit from 529f785 to 6398780

[nathanncohen]

comment:25

Okay, it looks like the automatic message does not work, but the branch is updated !

Nathann

[simon-king-jena]

comment:26

Replying to @vbraun:

Various doctests fail (see also patchbot)

WTF? When I reviewed it, things worked. Or did I accidentally merge other tickets that aren't dependencies for this?

[simon-king-jena]

comment:27

Hooray. After pulling from the ticket branch and merging with the current develop branch, make ptest results in

All tests passed!
----------------------------------------------------------------------
Total time for all tests: 3791.7 seconds
    cpu time: 6340.1 seconds
    cumulative wall time: 7304.6 seconds

Nathann's last commit looks like the right thing to do. Hence, I hope this time my positive review will persist...