Bezier pep8 #1182

Merged
merged 3 commits into from Sep 7, 2012

Projects

None yet

4 participants

@NelleV

In this branch, I did two things

  1. delete redundant code and code that wasn't called and couldn't work (see first commit)
  2. pep8 compliant

Thanks,
N

@travisbot

This pull request fails (merged 142e346a into cf7618c).

@pelson pelson commented on the diff Sep 1, 2012
lib/matplotlib/bezier.py
@@ -100,12 +95,9 @@ def split_de_casteljau(beta, t):
return left_beta, right_beta
-
-
-
-
-
-def find_bezier_t_intersecting_with_closedpath(bezier_point_at_t, inside_closedpath,
+# FIXME spelling mistake in the name of the parameter ``tolerence``
@pelson
pelson Sep 1, 2012

I suggest you just go ahead and do it.

@pelson
pelson Sep 1, 2012

Maybe I will backtrack on that one. Lets keep that particular change completely separate (and for 1.3 perhaps).

@pelson pelson commented on an outdated diff Sep 1, 2012
lib/matplotlib/bezier.py
@@ -129,16 +121,18 @@ def find_bezier_t_intersecting_with_closedpath(bezier_point_at_t, inside_closedp
end_inside = inside_closedpath(end)
if not xor(start_inside, end_inside):
- raise NonIntersectingPathException("the segment does not seemed to intersect with the path")
+ raise NonIntersectingPathException(
+ "the segment does not seemed to intersect with the path")
@pelson
pelson Sep 1, 2012

typo with the tense.

@pelson pelson commented on the diff Sep 1, 2012
lib/matplotlib/bezier.py
return path_left, path_right
-
-def make_wedged_bezier2(bezier2, length, shrink_factor=0.5):
@pelson
pelson Sep 1, 2012

So how do you know this isn't used? It could be that upstream users are using it...

@mdboom
mdboom Sep 7, 2012

I think the issue is that this function is included twice -- only the second copy below gets defined, so I think I'm cool with this change.

@pelson
Matplotlib Developers member

Comments aside. +1

@NelleV

Fixed

@pelson
Matplotlib Developers member

Typos fixed. Remaining discussion on the removal of make_wedged_bezier2.

@travisbot

This pull request fails (merged 9c99ad35 into cf7618c).

@NelleV

@pelson this method was overwritten later on in the file, hence the deletion.

@pelson
Matplotlib Developers member

@NelleV: That is the kind of answer I was hoping you might be able to give.

This PR has my +1.

@travisbot

This pull request fails (merged 182cd74 into a7aaa83).

@mdboom mdboom merged commit 9eab454 into matplotlib:master Sep 7, 2012

1 check failed

Details default The Travis build failed
@pelson
Matplotlib Developers member

Thanks @NelleV (and @mdboom for merging). Nice tidy up.

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