Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pep8 cleaning in distances methods #26823

Closed
dcoudert opened this issue Dec 4, 2018 · 15 comments
Closed

pep8 cleaning in distances methods #26823

dcoudert opened this issue Dec 4, 2018 · 15 comments

Comments

@dcoudert
Copy link
Contributor

dcoudert commented Dec 4, 2018

We do pep8 cleaning in methods all_paths, shortest_path, shortest_path_length, _check_weight_function, shortest_paths, shortest_path_lengths, shortest_path_all_pairs.

Component: graph theory

Author: David Coudert

Branch/Commit: 2ff0fd2

Reviewer: Vincent Klein

Issue created by migration from https://trac.sagemath.org/ticket/26823

@dcoudert dcoudert added this to the sage-8.5 milestone Dec 4, 2018
@dcoudert
Copy link
Contributor Author

dcoudert commented Dec 4, 2018

Branch: public/26823_clean_distance_methods

@dcoudert
Copy link
Contributor Author

dcoudert commented Dec 4, 2018

New commits:

9dd97bdtrac #26823: pep8 in distance methods

@dcoudert
Copy link
Contributor Author

dcoudert commented Dec 4, 2018

Commit: 9dd97bd

@dcoudert

This comment has been minimized.

@vinklein vinklein mannequin self-assigned this Dec 7, 2018
@vinklein
Copy link
Mannequin

vinklein mannequin commented Dec 7, 2018

comment:3

python remark:
I think there is a bad copy-paste line 15709.

        if not self.has_vertex(u):
            raise ValueError("vertex '{}' is not in the (di)graph".format(u))
        if not self.has_vertex(u):
            raise ValueError("vertex '{}' is not in the (di)graph".format(u))

pep8 remarks:

    1. Missing whitespace around operator:
      l 15278: s=start
    1. At least two spaces before inline comment:
      l 15289: s = next(act_path_iter[-1]) # try to get the next neighbor/successor, ...

l 15553: if u == v: # to avoid a NetworkX bug

l 15714: # to avoid a NetworkX bug

    1. Do not assign a lambda expression use a def:
      l 15551, 15724, 15947, 16167, 16450, 16490, 16516: weight_function = lambda e: e[2]

l 15564, 15736, 15949, 16169, 16481: weight_function = lambda e: 1

    1. Too many blank lines:
      l 15557
    1. Missing whitespace:
      l 15984: _,pred = ...

l 16492: dist[u],pred[u] = ...

@vinklein vinklein mannequin removed their assignment Dec 7, 2018
@vinklein
Copy link
Mannequin

vinklein mannequin commented Dec 7, 2018

Reviewer: Vincent Klein

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 7, 2018

Changed commit from 9dd97bd to 6c67fc0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 7, 2018

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

6c67fc0trac #26823: reviewer's comments

@dcoudert
Copy link
Contributor Author

dcoudert commented Dec 7, 2018

comment:7

I have implemented all your comments. Thank you.

@vinklein
Copy link
Mannequin

vinklein mannequin commented Dec 7, 2018

comment:8

You're welcome.

There is one remaining weight_function = lambda e: 1 in shortest_path_all_pairs line 16487.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 7, 2018

Changed commit from 6c67fc0 to 2ff0fd2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 7, 2018

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

2ff0fd2trac #26823: another lambda

@dcoudert
Copy link
Contributor Author

dcoudert commented Dec 7, 2018

comment:10

oups. Fixed.

@vbraun
Copy link
Member

vbraun commented Dec 23, 2018

Changed branch from public/26823_clean_distance_methods to 2ff0fd2

@embray
Copy link
Contributor

embray commented Dec 28, 2018

comment:13

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants