Opened 12 months ago
Closed 10 months ago
#27135 closed enhancement (fixed)
pep8 in digraph_generators.py (part 2)
Reported by:  dcoudert  Owned by:  

Priority:  minor  Milestone:  sage8.7 
Component:  graph theory  Keywords:  
Cc:  Merged in:  
Authors:  David Coudert  Reviewers:  Bryan Ginge Chen 
Report Upstream:  N/A  Work issues:  
Branch:  5960ed4 (Commits)  Commit:  5960ed413938c114d74499ef835a9d6efc4b4691 
Dependencies:  Stopgaps: 
Description (last modified by )
Clean
Complete
Circuit
Circulant
In both Circuit
and Circulant
, we also avoid creating the list of edges before adding it to the digraph (small speed up)
Change History (17)
comment:1 Changed 12 months ago by
 Branch set to u/dcoudert/27135_digraph_generators_2
 Commit set to aaf7d592fd474670912e02c879fa02eba9542ee5
 Status changed from new to needs_review
comment:2 Changed 12 months ago by
 Type changed from PLEASE CHANGE to enhancement
comment:3 Changed 12 months ago by
 Priority changed from major to minor
comment:4 Changed 12 months ago by
 Reviewers set to Bryan Ginge Chen
 Status changed from needs_review to needs_work
In the documentation of Circuit
, there are missing periods at the ends of sentences.
In Circulant
, the 3 occurrences of relative integers
should just be integers
(I guess this is a FrenchtoEnglish thing).
Feel free to set to positive review after fixing.
comment:5 Changed 12 months ago by
 Commit changed from aaf7d592fd474670912e02c879fa02eba9542ee5 to 3748045866e8d5412b8411d29b4da8f7d2199b0e
comment:6 Changed 12 months ago by
 Description modified (diff)
 Status changed from needs_work to positive_review
Thank you for the review.
comment:7 Changed 12 months ago by
Oh sorry, I edited my review comment last night and I guess you didn't see it. There are still missing periods at the ends of the sentences in the docstring of Circuit
. (Again, feel free to set to positive review afterwards).
comment:8 Changed 12 months ago by
 Status changed from positive_review to needs_work
comment:9 Changed 12 months ago by
 Commit changed from 3748045866e8d5412b8411d29b4da8f7d2199b0e to 39ce46496b62ebd63a8e480d8f992a0360947b7f
Branch pushed to git repo; I updated commit sha1. New commits:
39ce464  trac #27135: add missing periods in Circuit

comment:10 Changed 12 months ago by
 Status changed from needs_work to positive_review
I added the missing periods. Thank you for the review.
comment:11 Changed 12 months ago by
 Status changed from positive_review to needs_work
Merge conflict
comment:12 Changed 12 months ago by
OK. I guess I have to wait until next beta to find the issue.
comment:13 Changed 11 months ago by
 Commit changed from 39ce46496b62ebd63a8e480d8f992a0360947b7f to 5960ed413938c114d74499ef835a9d6efc4b4691
Branch pushed to git repo; I updated commit sha1. New commits:
5960ed4  trac #27135: fix merge conflict with 8.7.beta3

comment:14 Changed 11 months ago by
 Status changed from needs_work to needs_review
Rebased on 8.7.beta3 and fix merge conflict.
comment:15 Changed 11 months ago by
Can I set this ticket to positive review ?
comment:16 Changed 11 months ago by
 Status changed from needs_review to positive_review
Sorry that I missed this. It looks good to me!
comment:17 Changed 10 months ago by
 Branch changed from u/dcoudert/27135_digraph_generators_2 to 5960ed413938c114d74499ef835a9d6efc4b4691
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
trac #27135: clean digraph_generators part 2