Opened 12 months ago

Closed 12 months ago

Last modified 12 months ago

#26820 closed enhancement (fixed)

pep8 cleaning in eccentricity, center, radius, diameter, periphery

Reported by: dcoudert Owned by:
Priority: major Milestone: sage-8.6
Component: graph theory Keywords: py3, graph
Cc: Merged in:
Authors: David Coudert Reviewers: Vincent Klein
Report Upstream: N/A Work issues:
Branch: 63b1806 (Commits) Commit: 63b18061fc519b53b6c44ce364f363a7d8943a3f
Dependencies: Stopgaps:

Description

PEP8 cleaning in methods eccentricity, radius, diameter, center, periphery. On the way, we do:

  • in radius, center, periphery: replace the long description of the list of algorithms by a forward to method eccentricity
  • in radius, diameter, center, periphery: set parameter v when calling method eccentricity in order to avoid calls to .vertices()

Change History (9)

comment:1 Changed 12 months ago by dcoudert

  • Branch set to public/26820_eccentricity
  • Commit set to dd570a5bffd26e2411e18d8fc47edb33629badf6
  • Status changed from new to needs_review

New commits:

dd570a5trac #26820: clean eccentricity, radius, diameter, center, periphery

comment:2 Changed 12 months ago by vklein

Hi,

What is the goal of this ticket? Is it to enhance compatibility with pep8 or to be fully compatible with pep8?

In the second case i have the following remarks:

  • 1. line 14069: weight_function = lambda e: e[2]. Do not assign a lambda expression use a def.

Pep8 quote:

Always use a def statement instead of an assignment statement that binds a lambda
expression directly to an identifier.

Yes:

def f(x): return 2*x

No:

f = lambda x: 2*x

  • 2. lines 14122 and 14123: continuation line under-indented for visual indent

for example you can do this :

                length = self.shortest_path_lengths(u, by_weight=by_weight,
                                                    algorithm=algorithm,
                                                    weight_function=weight_function,
                                                    check_weight=check_weight)

instead of this :

                length = self.shortest_path_lengths(u, by_weight=by_weight,
                         algorithm=algorithm, weight_function=weight_function,
                         check_weight=check_weight)
  • 3. line 14135: Need two spaces before inline comment.
Last edited 12 months ago by vklein (previous) (diff)

comment:3 Changed 12 months ago by vklein

  • Reviewers set to Vincent Klein

comment:4 Changed 12 months ago by git

  • Commit changed from dd570a5bffd26e2411e18d8fc47edb33629badf6 to 63b18061fc519b53b6c44ce364f363a7d8943a3f

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

63b1806trac #26820: reviewer's comments

comment:5 Changed 12 months ago by dcoudert

I have implemented your comments. Thank you.

The goal is to do significant enhancements, and if possible to reach 100% pep8 compatibility (a lot to do for that).

comment:6 Changed 12 months ago by vklein

Green bot => positive review

comment:7 Changed 12 months ago by vklein

  • Status changed from needs_review to positive_review

comment:8 Changed 12 months ago by vbraun

  • Branch changed from public/26820_eccentricity to 63b18061fc519b53b6c44ce364f363a7d8943a3f
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:9 Changed 12 months ago by embray

  • Milestone changed from sage-8.5 to sage-8.6

This tickets were closed as fixed after the Sage 8.5 release.

Note: See TracTickets for help on using tickets.