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

autopep8 #30

Merged
merged 1 commit into from
Oct 23, 2018
Merged

autopep8 #30

merged 1 commit into from
Oct 23, 2018

Conversation

mlautman
Copy link
Member

@mlautman mlautman commented Oct 16, 2018

Results of running
autopep8 --max-line-length 120 --in-place $1 on each .py file

@@ -91,7 +93,8 @@ def __init__(self, bag_timeline):
self._history_top = 30

# Background Rendering
self._bag_end_color = QColor(0, 0, 0, 25) # color of background of timeline before first message and after last
self._bag_end_color = QColor(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe instead of breaking the code, you move the comment above this line.

@@ -605,7 +621,8 @@ def _draw_time_divisions(self, painter):
else:
major_division = min(major_divisions)

minor_divisions = [s for s in self._sec_divisions if x_per_sec * s >= self._minor_spacing and major_division % s == 0]
minor_divisions = [s for s in self._sec_divisions if x_per_sec *
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bad break, move the full if clause to the second line or something else more reasonable

@@ -617,7 +634,8 @@ def _draw_time_divisions(self, painter):
self._draw_major_divisions(painter, major_stamps, start_stamp, major_division)

if minor_division:
minor_stamps = [s for s in self._get_stamps(start_stamp, minor_division) if s not in major_stamps]
minor_stamps = [s for s in self._get_stamps(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bad break, move if to second line or something else reasonable

@@ -1132,7 +1159,8 @@ def on_mouse_move(self, event):
if dx_drag != 0:
self.translate_timeline(-self.map_dx_to_dstamp(dx_drag))
if (dx_drag == 0 and abs(dy_drag) > 0) or (dx_drag != 0 and abs(float(dy_drag) / dx_drag) > 0.2 and abs(dy_drag) > 1):
zoom = min(self._max_zoom_speed, max(self._min_zoom_speed, 1.0 + self._zoom_sensitivity * dy_drag))
zoom = min(self._max_zoom_speed, max(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's up to you, but would these min-maxes be easier to read if the max function call was also moved to the second line?

@@ -1164,8 +1192,10 @@ def on_mouse_move(self, event):
dx_drag = x - self._dragged_pos.x()
dstamp = self.map_dx_to_dstamp(dx_drag)

self._selected_left = max(self._start_stamp.to_sec(), min(self._end_stamp.to_sec(), self._selected_left + dstamp))
self._selected_right = max(self._start_stamp.to_sec(), min(self._end_stamp.to_sec(), self._selected_right + dstamp))
self._selected_left = max(self._start_stamp.to_sec(), min(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Samething with max-min here. Does it make more sense to move the min?

@@ -89,7 +92,8 @@ def draw_timeline_segment(self, painter, topic, stamp_start, stamp_end, x, y, wi
max_interval_thumbnail = self.timeline.map_dx_to_dstamp(self.thumbnail_combine_px)
max_interval_thumbnail = max(0.1, max_interval_thumbnail)
thumbnail_gap = 6
thumbnail_x, thumbnail_y, thumbnail_height = x + 1, y + 1, height - 2 - thumbnail_gap # leave 1px border
thumbnail_x, thumbnail_y, thumbnail_height = x + \
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Break after the =

@mlautman
Copy link
Member Author

Merging per @brawner's instructions

@mlautman mlautman merged commit f8cd6a8 into master Oct 23, 2018
@mlautman mlautman deleted the pep8 branch October 23, 2018 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants