-
Notifications
You must be signed in to change notification settings - Fork 117
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
Review/fixes (Sourcery refactored) #66
Conversation
self.display.FitAll() | ||
else: | ||
self.layer.hide() | ||
self.display.FitAll() | ||
|
||
self.display.FitAll() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function App.show_layer
refactored with the following changes:
- Hoist repeated code outside conditional statement (
hoist-statement-from-if
)
box = BRepPrimAPI_MakeBox(axes, 2.0 * half_x, 2.0 * half_y, 2.0 * half_z).Shape() | ||
return box | ||
return BRepPrimAPI_MakeBox( | ||
axes, 2.0 * half_x, 2.0 * half_y, 2.0 * half_z | ||
).Shape() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function convert_bnd_to_shape
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
print("Cube mass = %s" % mass) | ||
print(f"Cube mass = {mass}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function cube_inertia_properties
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
shp_idx = 1 | ||
for face in t.faces(): | ||
for shp_idx, face in enumerate(t.faces(), start=1): | ||
brepgprop_SurfaceProperties(face, props) | ||
face_surf = props.Mass() | ||
print("Surface for face nbr %i : %f" % (shp_idx, face_surf)) | ||
shp_idx += 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function shape_faces_surface
refactored with the following changes:
- Replace manual loop counter with call to enumerate (
convert-to-enumerate
)
assert not aspect.ShaderProgram() is None, "no shader program is null" | ||
assert aspect.ShaderProgram() is not None, "no shader program is null" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 67-67
refactored with the following changes:
- Simplify logical expression using De Morgan identities (
de-morgan
)
if buttons == QtCore.Qt.LeftButton and not modifiers == QtCore.Qt.ShiftModifier: | ||
if ( | ||
buttons == QtCore.Qt.LeftButton | ||
and modifiers != QtCore.Qt.ShiftModifier | ||
): | ||
self.current_action = ON_DYN_ROT | ||
|
||
# dynamic zoom | ||
elif ( | ||
buttons == QtCore.Qt.RightButton | ||
and not modifiers == QtCore.Qt.ShiftModifier | ||
and modifiers != QtCore.Qt.ShiftModifier | ||
): | ||
self.current_action = ON_DYN_ZOOM | ||
|
||
# dynamic panning | ||
elif buttons == QtCore.Qt.MidButton: | ||
self.current_action = ON_DYN_PAN | ||
|
||
# zoom window, overpaints rectangle | ||
elif buttons == QtCore.Qt.RightButton and modifiers == QtCore.Qt.ShiftModifier: | ||
elif buttons == QtCore.Qt.RightButton: | ||
self.current_action = ON_ZOOM_AREA | ||
|
||
# select area | ||
elif buttons == QtCore.Qt.LeftButton and modifiers == QtCore.Qt.ShiftModifier: | ||
elif buttons == QtCore.Qt.LeftButton: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function GLWidget.mouseMoveEvent
refactored with the following changes:
- Simplify logical expression using De Morgan identities [×2] (
de-morgan
) - Remove redundant conditional [×2] (
remove-redundant-if
)
This removes the following comments ( why? ):
# zoom window, overpaints rectangle
# dynamic zoom
# dynamic panning
# select area
if self._have_pyqt5: | ||
delta = event.angleDelta().y() | ||
else: | ||
delta = event.delta() | ||
|
||
if delta > 0: | ||
self.zoom_factor = 1.3 | ||
else: | ||
self.zoom_factor = 0.7 | ||
delta = event.angleDelta().y() if self._have_pyqt5 else event.delta() | ||
self.zoom_factor = 1.3 if delta > 0 else 0.7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function GLWidget.wheelEvent
refactored with the following changes:
- Replace if statement with if expression [×2] (
assign-if-exp
)
if abs(dx) <= tolerance and abs(dy) <= tolerance: | ||
# zooming at a near nil value can segfault the opengl viewer | ||
pass | ||
else: | ||
if not self.is_right_mouse_button_surpressed: | ||
coords = [ | ||
self.point_on_mouse_press[0], | ||
self.point_on_mouse_press[1], | ||
self.point_on_mouse_move[0], | ||
self.point_on_mouse_move[1], | ||
] | ||
self._display.ZoomArea(*coords) | ||
if ( | ||
abs(dx) > tolerance or abs(dy) > tolerance | ||
) and not self.is_right_mouse_button_surpressed: | ||
coords = [ | ||
self.point_on_mouse_press[0], | ||
self.point_on_mouse_press[1], | ||
self.point_on_mouse_move[0], | ||
self.point_on_mouse_move[1], | ||
] | ||
self._display.ZoomArea(*coords) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function GLWidget.on_zoom_area
refactored with the following changes:
- Swap if/else to remove empty if body (
remove-pass-body
) - Merge nested if conditions (
merge-nested-ifs
)
This removes the following comments ( why? ):
# zooming at a near nil value can segfault the opengl viewer
perform_action = False | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function GLWidget._dispatch_camera_command_actions
refactored with the following changes:
- Move assignment closer to its usage within a block (
move-assign-in-block
) - Inline variable that is immediately returned (
inline-immediately-returned-variable
)
if abs(dx) <= tolerance and abs(dy) <= tolerance: | ||
pass | ||
|
||
else: | ||
if abs(dx) > tolerance or abs(dy) > tolerance: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function GLWidget.drawBox
refactored with the following changes:
- Swap if/else to remove empty if body (
remove-pass-body
)
Pull Request #65 refactored by Sourcery.
If you're happy with these changes, merge this Pull Request using the Squash and merge strategy.
NOTE: As code is pushed to the original Pull Request, Sourcery will
re-run and update (force-push) this Pull Request with new refactorings as
necessary. If Sourcery finds no refactorings at any point, this Pull Request
will be closed automatically.
See our documentation here.
Run Sourcery locally
Reduce the feedback loop during development by using the Sourcery editor plugin:
Review changes via command line
To manually merge these changes, make sure you're on the
review/fixes
branch, then run:Help us improve this pull request!