#27628 closed enhancement (fixed)
py3: fix doctest with igraph
Reported by: | dcoudert | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.8 |
Component: | graph theory | Keywords: | py3, graph |
Cc: | dimpase | Merged in: | |
Authors: | David Coudert | Reviewers: | Frédéric Chapoton, Dima Pasechnik |
Report Upstream: | N/A | Work issues: | |
Branch: | 1e773f5 (Commits) | Commit: | |
Dependencies: | Stopgaps: |
Description (last modified by )
ordering issue.
Also change the way to check if igraph is installed to avoid a recurrent pyflakes error.
Change History (17)
comment:1 Changed 19 months ago by
- Branch set to public/graphs/27628_igraph_graph
- Commit set to 8ee4a7aab47099b5586311d36ded3c45076eee6c
- Status changed from new to needs_review
comment:2 Changed 19 months ago by
- Reviewers set to Frédéric Chapoton
- Status changed from needs_review to positive_review
ok, looks good enough
comment:3 Changed 19 months ago by
Thank you Frederic for this super fast review ;)
comment:4 Changed 19 months ago by
- Status changed from positive_review to needs_work
There are a couple of failures of teh sort
********************************************************************** File "src/sage/graphs/generic_graph.py", line 9581, in sage.graphs.generic_graph.GenericGraph.? Failed example: G.pagerank(algorithm="igraph") Exception raised: Traceback (most recent call last): File "/var/lib/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 671, in _run self.compile_and_execute(example, compiler, test.globs) File "/var/lib/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 1095, in compile_and_execute exec(compiled, globs) File "<doctest sage.graphs.generic_graph.GenericGraph.?[10]>", line 1, in <module> G.pagerank(algorithm="igraph") File "/var/lib/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/graphs/generic_graph.py", line 9668, in pagerank raise PackageNotFoundError("igraph") PackageNotFoundError: the package 'igraph' was not found. You can install it by running 'sage -i igraph' in a shell **********************************************************************
comment:5 Changed 19 months ago by
It's not due to this ticket (nothing to do with pagerank
) but to #27480 which adds pagerank
. We forgot to tag doctests as # optional - python_igraph
, and I believe it's too late so we must wait for next beta, right ?
comment:6 Changed 19 months ago by
- Status changed from needs_work to needs_review
Can we set this ticket back to positive review ?
comment:7 Changed 18 months ago by
- Commit changed from 8ee4a7aab47099b5586311d36ded3c45076eee6c to 1e773f5bfbd2fbe0712c7fe0e1731eeb7d664dc5
comment:8 Changed 18 months ago by
I changed the way we check if igraph is installed to avoid a pyflakes error.
comment:9 Changed 18 months ago by
- Cc dimpase added
- Description modified (diff)
comment:10 Changed 18 months ago by
- Reviewers changed from Frédéric Chapoton to Frédéric Chapoton, Dima Pasechnik
- Status changed from needs_review to positive_review
looks good to me.
comment:11 Changed 18 months ago by
Well, let's do a follow up ticket since this is getting merged properly right now. Please refrain to introduce anymore call to is_package_installed
, especially when checking for a python package. There is a proper python way to do this, like in the other sections of code using igraph
- from sage/graphs/digraph.py
line 693 onward
try: import igraph except ImportError: raise ImportError(...)
replacing the ImportError(...)
with PackageNotFoundError
is acceptable.
is_package_installed
is a big problem for sage-on-distro and it is a filthy quick and dirty hack that shouldn't ever have been included.
comment:12 Changed 18 months ago by
how about a ticket for getting rid of is_package_installed()
? Or at least making sure it's not used on importable things?
I wonder what pyflakes error one sees on a seemingly good try/except.
comment:13 Changed 18 months ago by
It is already on its way out. The only place it is not replaced by feature or something else at runtime is in sage/databases/cremona.py
and there is a ticket for that.
It is currently impossible to totally remove it. doctesting with --option=all
relies on it (or other features of packages.py
which is just as bad) :( and building with some optional package still depends on it look in sage_setup/optional_extension.py
. In both case replacement are not easy.
comment:14 Changed 18 months ago by
- Branch changed from public/graphs/27628_igraph_graph to 1e773f5bfbd2fbe0712c7fe0e1731eeb7d664dc5
- Resolution set to fixed
- Status changed from positive_review to closed
comment:15 Changed 18 months ago by
- Commit 1e773f5bfbd2fbe0712c7fe0e1731eeb7d664dc5 deleted
There is an issue with igraph on the arando patchbot with 8.8.b4 and 8.8.b5:
********************************************************************** File "src/sage/graphs/generic_graph.py", line 9663, in sage.graphs.generic_graph.GenericGraph.? Failed example: G.pagerank(alpha=0.50, algorithm="igraph") # optional - python_igraph Expected: {0: 0.25, 1: 0.25, 2: 0.24999999999999997, 3: 0.24999999999999997} Got: {0: 0.25, 1: 0.25, 2: 0.25, 3: 0.25} ********************************************************************** 1 item had failures: 1 of 860 in sage.graphs.generic_graph.GenericGraph.? [3497 tests, 1 failure, 50.96 s] ---------------------------------------------------------------------- sage -t --long src/sage/graphs/generic_graph.py # 1 doctest failed
comment:16 Changed 18 months ago by
one can use a tag like # abs tol 1e-12
and make the data
{0: 0.25, 1: 0.25, 2: 0.25, 3: 0.25}
comment:17 Changed 18 months ago by
it's what we do in #27811 ;)
New commits:
trac #27628: doctest with igraph