Opened 10 months ago

Closed 8 months ago

#27135 closed enhancement (fixed)

pep8 in digraph_generators.py (part 2)

Reported by: dcoudert Owned by:
Priority: minor Milestone: sage-8.7
Component: graph theory Keywords:
Cc: Merged in:
Authors: David Coudert Reviewers: Bryan Gin-ge Chen
Report Upstream: N/A Work issues:
Branch: 5960ed4 (Commits) Commit: 5960ed413938c114d74499ef835a9d6efc4b4691
Dependencies: Stopgaps:

Description (last modified by dcoudert)

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 10 months ago by dcoudert

  • Authors set to David Coudert
  • Branch set to u/dcoudert/27135_digraph_generators_2
  • Commit set to aaf7d592fd474670912e02c879fa02eba9542ee5
  • Status changed from new to needs_review

New commits:

aaf7d59trac #27135: clean digraph_generators part 2

comment:2 Changed 10 months ago by dcoudert

  • Type changed from PLEASE CHANGE to enhancement

comment:3 Changed 10 months ago by dcoudert

  • Priority changed from major to minor

comment:4 Changed 10 months ago by gh-bryangingechen

  • Reviewers set to Bryan Gin-ge 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 French-to-English thing).

Feel free to set to positive review after fixing.

Last edited 10 months ago by gh-bryangingechen (previous) (diff)

comment:5 Changed 10 months ago by git

  • Commit changed from aaf7d592fd474670912e02c879fa02eba9542ee5 to 3748045866e8d5412b8411d29b4da8f7d2199b0e

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

2ac1095trac #27135: Merged with 8.7.beta1
3748045trac #27135: remove occurence of relative

comment:6 Changed 10 months ago by dcoudert

  • Description modified (diff)
  • Status changed from needs_work to positive_review

Thank you for the review.

comment:7 Changed 10 months ago by gh-bryangingechen

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 10 months ago by gh-bryangingechen

  • Status changed from positive_review to needs_work

comment:9 Changed 10 months ago by git

  • Commit changed from 3748045866e8d5412b8411d29b4da8f7d2199b0e to 39ce46496b62ebd63a8e480d8f992a0360947b7f

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

39ce464trac #27135: add missing periods in Circuit

comment:10 Changed 10 months ago by dcoudert

  • Status changed from needs_work to positive_review

I added the missing periods. Thank you for the review.

comment:11 Changed 9 months ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:12 Changed 9 months ago by dcoudert

OK. I guess I have to wait until next beta to find the issue.

comment:13 Changed 9 months ago by git

  • Commit changed from 39ce46496b62ebd63a8e480d8f992a0360947b7f to 5960ed413938c114d74499ef835a9d6efc4b4691

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

5960ed4trac #27135: fix merge conflict with 8.7.beta3

comment:14 Changed 9 months ago by dcoudert

  • Status changed from needs_work to needs_review

Rebased on 8.7.beta3 and fix merge conflict.

comment:15 Changed 8 months ago by dcoudert

Can I set this ticket to positive review ?

comment:16 Changed 8 months ago by gh-bryangingechen

  • Status changed from needs_review to positive_review

Sorry that I missed this. It looks good to me!

comment:17 Changed 8 months ago by vbraun

  • Branch changed from u/dcoudert/27135_digraph_generators_2 to 5960ed413938c114d74499ef835a9d6efc4b4691
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.