Opened 4 years ago
Closed 4 years ago
#26621 closed enhancement (fixed)
clean bliss.pyx
Reported by: | dcoudert | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.5 |
Component: | graph theory | Keywords: | py3, graph |
Cc: | tscrim, dimpase | Merged in: | |
Authors: | David Coudert | Reviewers: | Travis Scrimshaw, Dima Pasechnik |
Report Upstream: | N/A | Work issues: | |
Branch: | 92c3509 (Commits, GitHub, GitLab) | Commit: | 92c3509f81a65bd705ece438714215c845552b62 |
Dependencies: | Stopgaps: |
Description
PEP8 and avoid using .edges()
and .vertices()
Change History (10)
comment:1 Changed 4 years ago by
- Branch set to public/26621_bliss
- Cc tscrim dima added
- Commit set to 9e0691ee5c7897f8d4ceca97e23c6bb106ed9992
- Status changed from new to needs_review
comment:2 Changed 4 years ago by
- Cc dimpase added; dima removed
Dima, see comment:1.
David, Micro-optimization, but since you know cdef list done = [False] * n
will be a length n
array of bint
's, you are better to directly allocate such an array (Cython generates better C code and you do not have the overhead of list
).
comment:3 Changed 4 years ago by
- Status changed from needs_review to needs_work
I will do that. I also did a mistake in initializations. I will fix that.
comment:4 Changed 4 years ago by
- Commit changed from 9e0691ee5c7897f8d4ceca97e23c6bb106ed9992 to 1a59d4f412a3a0c2919a4c6f0846bcafb10c679a
Branch pushed to git repo; I updated commit sha1. New commits:
1a59d4f | trac #26621: improvements
|
comment:6 Changed 4 years ago by
It seems to be a good idea to declare logLnr
as
cdef int logLnr = 0
(in two places in src/sage/graphs/bliss.pyx
)
comment:7 Changed 4 years ago by
- Commit changed from 1a59d4f412a3a0c2919a4c6f0846bcafb10c679a to 92c3509f81a65bd705ece438714215c845552b62
Branch pushed to git repo; I updated commit sha1. New commits:
92c3509 | trac #26621: initialize logLnr to 0
|
comment:8 Changed 4 years ago by
It's certainly better like that. Thanks.
comment:9 Changed 4 years ago by
- Reviewers set to Travis Scrimshaw, Dima Pasechnik
- Status changed from needs_review to positive_review
Thank you Dima. Everything else LGTM.
comment:10 Changed 4 years ago by
- Branch changed from public/26621_bliss to 92c3509f81a65bd705ece438714215c845552b62
- Resolution set to fixed
- Status changed from positive_review to closed
@dima: could you check that variable
logLnr
is always assigned a value in methodsbliss_graph_from_labelled_edges
andbliss_digraph_from_labelled_edges
. I seems that it might be used uninitialized...New commits:
trac #26621: clean bliss.pyx