Opened 6 months ago

Closed 6 months ago

Last modified 6 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 6 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 6 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 6 months ago by vklein (previous) (diff)

comment:3 Changed 6 months ago by vklein

  • Reviewers set to Vincent Klein

comment:4 Changed 6 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 6 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 6 months ago by vklein

Green bot => positive review

comment:7 Changed 6 months ago by vklein

  • Status changed from needs_review to positive_review

comment:8 Changed 6 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 6 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.