From 60b9a5018d91f4e4525c19db0660bed7df59596f Mon Sep 17 00:00:00 2001 From: Ami Fischman Date: Mon, 13 Nov 2023 15:21:36 -0800 Subject: [PATCH] Misc pylint fixes. Down to 130 findings. --- .pylintrc | 3 ++ src/build123d/__init__.py | 1 - src/build123d/_dev/scm_version.py | 2 +- src/build123d/build_common.py | 5 +- src/build123d/build_sketch.py | 4 +- src/build123d/drafting.py | 13 +++-- src/build123d/exporters.py | 26 +++++----- src/build123d/geometry.py | 17 +++++++ src/build123d/joints.py | 11 +++-- src/build123d/jupyter_tools.py | 14 +++--- src/build123d/objects_sketch.py | 16 ++----- src/build123d/operations_generic.py | 73 ++++++++++++++--------------- src/build123d/operations_part.py | 1 + src/build123d/operations_sketch.py | 4 +- src/build123d/topology.py | 29 ++++-------- src/build123d/version.py | 11 +++-- 16 files changed, 117 insertions(+), 113 deletions(-) diff --git a/.pylintrc b/.pylintrc index bd99392..e5469e8 100644 --- a/.pylintrc +++ b/.pylintrc @@ -9,3 +9,6 @@ disable= protected-access, # _variable to be hiddened from external users too-many-arguments, # CAD is complex too-few-public-methods # Objects and Operations will not have methods outside of _init + +ignore-paths= + ./src/build123d/_version.py # Generated diff --git a/src/build123d/__init__.py b/src/build123d/__init__.py index 3cf3fc9..0c1dec0 100644 --- a/src/build123d/__init__.py +++ b/src/build123d/__init__.py @@ -19,7 +19,6 @@ from build123d.pack import * from build123d.topology import * from build123d.drafting import * from build123d.persistence import modify_copyreg -from build123d.drafting import * from .version import version as __version__ diff --git a/src/build123d/_dev/scm_version.py b/src/build123d/_dev/scm_version.py index cea27e5..38c8e25 100644 --- a/src/build123d/_dev/scm_version.py +++ b/src/build123d/_dev/scm_version.py @@ -2,7 +2,7 @@ import os.path as pth try: - from setuptools_scm import get_version + from setuptools_scm import get_version # pylint: disable=import-error version = get_version(root=pth.join("..", "..", ".."), relative_to=__file__) except Exception as exc: diff --git a/src/build123d/build_common.py b/src/build123d/build_common.py index 5360d96..a482b97 100644 --- a/src/build123d/build_common.py +++ b/src/build123d/build_common.py @@ -593,8 +593,7 @@ class Builder(ABC): faces = self.faces(select) face_count = len(faces) if face_count != 1: - msg = f"Found {face_count} faces, returning first" - warnings.warn(msg) + warnings.warn(f"Found {face_count} faces, returning first") return faces[0] def solids(self, select: Select = Select.ALL) -> ShapeList[Solid]: @@ -907,7 +906,7 @@ class PolarLocations(LocationList): ): if count < 1: raise ValueError(f"At least 1 elements required, requested {count}") - elif count == 1: + if count == 1: angle_step = 0 else: angle_step = angular_range / (count - int(endpoint)) diff --git a/src/build123d/build_sketch.py b/src/build123d/build_sketch.py index a93ed84..742b79c 100644 --- a/src/build123d/build_sketch.py +++ b/src/build123d/build_sketch.py @@ -108,6 +108,8 @@ class BuildSketch(Builder): wires = Wire.combine(self.pending_edges) return wires if len(wires) > 1 else wires[0] - def _add_to_pending(self, *objects: Edge): + def _add_to_pending(self, *objects: Edge, face_plane: Plane = None): """Integrate a sequence of objects into existing builder object""" + if face_plane: + raise NotImplementedError("face_plane arg not supported for this method") self.pending_edges.extend(objects) diff --git a/src/build123d/drafting.py b/src/build123d/drafting.py index 0407bfc..a7044bb 100644 --- a/src/build123d/drafting.py +++ b/src/build123d/drafting.py @@ -44,12 +44,12 @@ from build123d.build_enums import ( ) from build123d.build_line import BuildLine from build123d.build_sketch import BuildSketch -from build123d.geometry import Axis, Color, Location, Plane, Pos, Vector, VectorLike +from build123d.geometry import Axis, Location, Plane, Pos, Vector, VectorLike from build123d.objects_curve import Line, TangentArc from build123d.objects_sketch import BaseSketchObject, Polygon, Text from build123d.operations_generic import fillet, mirror, sweep from build123d.operations_sketch import make_face, trace -from build123d.topology import Compound, Curve, Edge, Sketch, Vertex, Wire +from build123d.topology import Compound, Edge, Sketch, Vertex, Wire class ArrowHead(BaseSketchObject): @@ -177,6 +177,7 @@ class Draft: Defaults to 2.0. """ + # pylint: disable=too-many-instance-attributes # Class Attributes unit_LUT: ClassVar[dict] = {True: "mm", False: '"'} @@ -371,6 +372,8 @@ class DimensionLine(BaseSketchObject): label_angle: bool = False, mode: Mode = Mode.ADD, ) -> Sketch: + # pylint: disable=too-many-locals + context = BuildSketch._get_context(self) if sketch is None and not (context is None or context.sketch is None): sketch = context.sketch @@ -398,7 +401,7 @@ class DimensionLine(BaseSketchObject): # Calculate the arrow shaft length for up to three types if arrows.count(True) == 0: raise ValueError("No output - no arrows selected") - elif label_length + arrows.count(True) * draft.arrow_length < path_length: + if label_length + arrows.count(True) * draft.arrow_length < path_length: shaft_length = (path_length - label_length) / 2 - draft.pad_around_text shaft_pair = [ path_obj.trim(0.0, shaft_length / path_length), @@ -506,6 +509,8 @@ class ExtensionLine(BaseSketchObject): project_line: VectorLike = None, mode: Mode = Mode.ADD, ): + # pylint: disable=too-many-locals + context = BuildSketch._get_context(self) if sketch is None and not (context is None or context.sketch is None): sketch = context.sketch @@ -620,6 +625,8 @@ class TechnicalDrawing(BaseSketchObject): line_width: float = 0.5, mode: Mode = Mode.ADD, ): + # pylint: disable=too-many-locals + page_dim = TechnicalDrawing.page_sizes[page_size] # Frame frame_width = page_dim[0] - 2 * TechnicalDrawing.margin - 2 * nominal_text_size diff --git a/src/build123d/exporters.py b/src/build123d/exporters.py index 8f28c3c..203d10e 100644 --- a/src/build123d/exporters.py +++ b/src/build123d/exporters.py @@ -38,17 +38,6 @@ from copy import copy import ezdxf import svgpathtools as PT -from build123d.topology import ( - BoundBox, - Compound, - Edge, - Wire, - GeomType, - Shape, - Vector, - VectorLike, -) -from build123d.build_enums import Unit from ezdxf import zoom from ezdxf.colors import RGB, aci2rgb from ezdxf.math import Vec2 @@ -65,6 +54,18 @@ from OCP.TopAbs import TopAbs_Orientation, TopAbs_ShapeEnum # type: ignore from OCP.TopExp import TopExp_Explorer # type: ignore from typing_extensions import Self +from build123d.topology import ( + BoundBox, + Compound, + Edge, + Wire, + GeomType, + Shape, + Vector, + VectorLike, +) +from build123d.build_enums import Unit + PathSegment = Union[PT.Line, PT.Arc, PT.QuadraticBezier, PT.CubicBezier] # --------------------------------------------------------------------------- @@ -893,8 +894,7 @@ class ExportSVG(Export2D): aci2rgb() function. We prefer (0,0,0).""" if ci == ColorIndex.BLACK: return (0, 0, 0) - else: - return aci2rgb(ci.value) + return aci2rgb(ci.value) if isinstance(fill_color, ColorIndex): fill_color = color_from_index(fill_color) diff --git a/src/build123d/geometry.py b/src/build123d/geometry.py index b127183..8a69829 100644 --- a/src/build123d/geometry.py +++ b/src/build123d/geometry.py @@ -69,6 +69,10 @@ from OCP.Quantity import Quantity_ColorRGBA from OCP.TopLoc import TopLoc_Location from OCP.TopoDS import TopoDS_Face, TopoDS_Shape +from build123d.build_enums import ( + Align, +) + # Create a build123d logger to distinguish these logs from application logs. # If the user doesn't configure logging, all build123d logs will be discarded. logging.getLogger("build123d").addHandler(logging.NullHandler()) @@ -828,6 +832,19 @@ class BoundBox: and second_box.max.Z < self.max.Z ) + def to_align_offset(self, align: Tuple[float, float]) -> Tuple[float, float]: + align_offset = [] + for i in range(2): + if align[i] == Align.MIN: + align_offset.append(-self.min.to_tuple()[i]) + elif align[i] == Align.CENTER: + align_offset.append( + -(self.min.to_tuple()[i] + self.max.to_tuple()[i]) / 2 + ) + elif align[i] == Align.MAX: + align_offset.append(-self.max.to_tuple()[i]) + return align_offset + class Color: """ diff --git a/src/build123d/joints.py b/src/build123d/joints.py index a975d17..6993721 100644 --- a/src/build123d/joints.py +++ b/src/build123d/joints.py @@ -87,7 +87,7 @@ class RigidJoint(Joint): super().__init__(label, to_part) @overload - def connect_to(self, other: BallJoint, *, angles: RotationLike = None): + def connect_to(self, other: BallJoint, *, angles: RotationLike = None, **kwargs): """Connect RigidJoint and BallJoint""" @overload @@ -154,7 +154,8 @@ class RigidJoint(Joint): position (float, optional): linear position. Defaults to linear range min. Raises: - TypeError: other must of type BallJoint, CylindricalJoint, LinearJoint, RevoluteJoint, RigidJoint + TypeError: other must be of a type in: BallJoint, CylindricalJoint, + LinearJoint, RevoluteJoint, RigidJoint. """ if isinstance(other, RigidJoint): @@ -184,8 +185,9 @@ class RigidJoint(Joint): other_location = other.relative_to(self, angles=angles).inverse() else: raise TypeError( - f"other must one of type BallJoint, CylindricalJoint, LinearJoint, RevoluteJoint, RigidJoint" - f" not {type(other)}" + "other must one of type " + "BallJoint, CylindricalJoint, LinearJoint, RevoluteJoint, RigidJoint " + f"not {type(other)}" ) return other_location @@ -490,6 +492,7 @@ class CylindricalJoint(Joint): Raises: ValueError: angle_reference must be normal to axis """ + # pylint: disable=too-many-instance-attributes @property def symbol(self) -> Compound: diff --git a/src/build123d/jupyter_tools.py b/src/build123d/jupyter_tools.py index 757f606..8ade2b1 100644 --- a/src/build123d/jupyter_tools.py +++ b/src/build123d/jupyter_tools.py @@ -220,14 +220,12 @@ def display(shape: Any) -> Javascript: if not hasattr(shape, "wrapped"): # Is a "Shape" raise ValueError(f"Type {type(shape)} is not supported") - payload.append( - dict( - shape=to_vtkpoly_string(shape), - color=DEFAULT_COLOR, - position=[0, 0, 0], - orientation=[0, 0, 0], - ) - ) + payload.append({ + "shape": to_vtkpoly_string(shape), + "color": DEFAULT_COLOR, + "position": [0, 0, 0], + "orientation": [0, 0, 0], + }) code = TEMPLATE.format(data=dumps(payload), element="element", ratio=0.5) return Javascript(code) diff --git a/src/build123d/objects_sketch.py b/src/build123d/objects_sketch.py index 1027d13..56d63ff 100644 --- a/src/build123d/objects_sketch.py +++ b/src/build123d/objects_sketch.py @@ -61,18 +61,7 @@ class BaseSketchObject(Sketch): ): if align is not None: align = tuplify(align, 2) - bbox = obj.bounding_box() - align_offset = [] - for i in range(2): - if align[i] == Align.MIN: - align_offset.append(-bbox.min.to_tuple()[i]) - elif align[i] == Align.CENTER: - align_offset.append( - -(bbox.min.to_tuple()[i] + bbox.max.to_tuple()[i]) / 2 - ) - elif align[i] == Align.MAX: - align_offset.append(-bbox.max.to_tuple()[i]) - obj.move(Location(Vector(*align_offset))) + obj.move(Location(Vector(*obj.bounding_box().to_align_offset(align)))) context: BuildSketch = BuildSketch._get_context(self, log=False) if context is None: @@ -298,6 +287,7 @@ class RegularPolygon(BaseSketchObject): align: tuple[Align, Align] = (Align.CENTER, Align.CENTER), mode: Mode = Mode.ADD, ): + # pylint: disable=too-many-locals context = BuildSketch._get_context(self) validate_inputs(context, self) @@ -528,7 +518,7 @@ class Text(BaseSketchObject): rotation (float, optional): angles to rotate objects. Defaults to 0. mode (Mode, optional): combination mode. Defaults to Mode.ADD. """ - + # pylint: disable=too-many-instance-attributes _applies_to = [BuildSketch._tag] def __init__( diff --git a/src/build123d/operations_generic.py b/src/build123d/operations_generic.py index 140f05c..6f7d359 100644 --- a/src/build123d/operations_generic.py +++ b/src/build123d/operations_generic.py @@ -230,23 +230,23 @@ def bounding_box( if context is not None: context._add_to_context(*new_faces, mode=mode) return Sketch(Compound.make_compound(new_faces).wrapped) - else: - new_objects = [] - for obj in object_list: - if isinstance(obj, Vertex): - continue - bbox = obj.bounding_box() - new_objects.append( - Solid.make_box( - bbox.size.X, - bbox.size.Y, - bbox.size.Z, - Plane((bbox.min.X, bbox.min.Y, bbox.min.Z)), - ) + + new_objects = [] + for obj in object_list: + if isinstance(obj, Vertex): + continue + bbox = obj.bounding_box() + new_objects.append( + Solid.make_box( + bbox.size.X, + bbox.size.Y, + bbox.size.Z, + Plane((bbox.min.X, bbox.min.Y, bbox.min.Z)), ) - if context is not None: - context._add_to_context(*new_objects, mode=mode) - return Part(Compound.make_compound(new_objects).wrapped) + ) + if context is not None: + context._add_to_context(*new_objects, mode=mode) + return Part(Compound.make_compound(new_objects).wrapped) #:TypeVar("ChamferFilletType"): Type of objects which can be chamfered or filleted @@ -325,7 +325,7 @@ def chamfer( context._add_to_context(new_part, mode=Mode.REPLACE) return Part(Compound.make_compound([new_part]).wrapped) - elif target._dim == 2: + if target._dim == 2: # Convert BaseSketchObject into Sketch so casting into Sketch during construction works target = ( Sketch(target.wrapped) if isinstance(target, BaseSketchObject) else target @@ -347,7 +347,7 @@ def chamfer( context._add_to_context(new_sketch, mode=Mode.REPLACE) return new_sketch - elif target._dim == 1: + if target._dim == 1: target = ( Wire(target.wrapped) if isinstance(target, BaseLineObject) @@ -420,7 +420,7 @@ def fillet( context._add_to_context(new_part, mode=Mode.REPLACE) return Part(Compound.make_compound([new_part]).wrapped) - elif target._dim == 2: + if target._dim == 2: # Convert BaseSketchObject into Sketch so casting into Sketch during construction works target = ( Sketch(target.wrapped) if isinstance(target, BaseSketchObject) else target @@ -441,7 +441,7 @@ def fillet( context._add_to_context(new_sketch, mode=Mode.REPLACE) return new_sketch - elif target._dim == 1: + if target._dim == 1: target = ( Wire(target.wrapped) if isinstance(target, BaseLineObject) @@ -508,12 +508,11 @@ def mirror( mirrored_compound = Compound.make_compound(mirrored) if all([obj._dim == 3 for obj in object_list]): return Part(mirrored_compound.wrapped) - elif all([obj._dim == 2 for obj in object_list]): + if all([obj._dim == 2 for obj in object_list]): return Sketch(mirrored_compound.wrapped) - elif all([obj._dim == 1 for obj in object_list]): + if all([obj._dim == 1 for obj in object_list]): return Curve(mirrored_compound.wrapped) - else: - return mirrored_compound + return mirrored_compound #:TypeVar("OffsetType"): Type of objects which can be offset @@ -641,12 +640,11 @@ def offset( offset_compound = Compound.make_compound(new_objects) if all([obj._dim == 3 for obj in object_list]): return Part(offset_compound.wrapped) - elif all([obj._dim == 2 for obj in object_list]): + if all([obj._dim == 2 for obj in object_list]): return Sketch(offset_compound.wrapped) - elif all([obj._dim == 1 for obj in object_list]): + if all([obj._dim == 1 for obj in object_list]): return Curve(offset_compound.wrapped) - else: - return offset_compound + return offset_compound #:TypeVar("ProjectType"): Type of objects which can be projected @@ -693,7 +691,7 @@ def project( if not objects and context is None: raise ValueError("No object to project") - elif not objects and context is not None and isinstance(context, BuildPart): + if not objects and context is not None and isinstance(context, BuildPart): object_list = context.pending_edges + context.pending_faces context.pending_edges = [] context.pending_faces = [] @@ -870,12 +868,11 @@ def scale( scale_compound = Compound.make_compound(new_objects) if all([obj._dim == 3 for obj in object_list]): return Part(scale_compound.wrapped) - elif all([obj._dim == 2 for obj in object_list]): + if all([obj._dim == 2 for obj in object_list]): return Sketch(scale_compound.wrapped) - elif all([obj._dim == 1 for obj in object_list]): + if all([obj._dim == 1 for obj in object_list]): return Curve(scale_compound.wrapped) - else: - return scale_compound + return scale_compound #:TypeVar("SplitType"): Type of objects which can be offset @@ -925,12 +922,11 @@ def split( split_compound = Compound.make_compound(new_objects) if all([obj._dim == 3 for obj in object_list]): return Part(split_compound.wrapped) - elif all([obj._dim == 2 for obj in object_list]): + if all([obj._dim == 2 for obj in object_list]): return Sketch(split_compound.wrapped) - elif all([obj._dim == 1 for obj in object_list]): + if all([obj._dim == 1 for obj in object_list]): return Curve(split_compound.wrapped) - else: - return split_compound + return split_compound #:TypeVar("SweepType"): Type of objects which can be swept @@ -1051,5 +1047,4 @@ def sweep( if new_solids: return Part(Compound.make_compound(new_solids).wrapped) - else: - return Sketch(Compound.make_compound(new_faces).wrapped) + return Sketch(Compound.make_compound(new_faces).wrapped) diff --git a/src/build123d/operations_part.py b/src/build123d/operations_part.py index 39564a3..7b8ef00 100644 --- a/src/build123d/operations_part.py +++ b/src/build123d/operations_part.py @@ -80,6 +80,7 @@ def extrude( Returns: Part: extruded object """ + # pylint: disable=too-many-locals context: BuildPart = BuildPart._get_context("extrude") validate_inputs(context, "extrude", to_extrude) diff --git a/src/build123d/operations_sketch.py b/src/build123d/operations_sketch.py index 6f466bc..79a6197 100644 --- a/src/build123d/operations_sketch.py +++ b/src/build123d/operations_sketch.py @@ -117,8 +117,8 @@ def trace( Convert edges, wires or pending edges into faces by sweeping a perpendicular line along them. Args: - lines (Union[Curve, Edge, Wire, Iterable[Union[Curve, Edge, Wire]]], optional): lines to trace. - Defaults to sketch pending edges. + lines (Union[Curve, Edge, Wire, Iterable[Union[Curve, Edge, Wire]]], optional): lines to + trace. Defaults to sketch pending edges. line_width (float, optional): Defaults to 1. mode (Mode, optional): combination mode. Defaults to Mode.ADD. diff --git a/src/build123d/topology.py b/src/build123d/topology.py index 94e2c1b..f17f70e 100644 --- a/src/build123d/topology.py +++ b/src/build123d/topology.py @@ -30,6 +30,7 @@ from __future__ import annotations # pylint has trouble with the OCP imports # pylint: disable=no-name-in-module, import-error +# pylint: disable=too-many-lines # other pylint warning to temp remove: # too-many-arguments, too-many-locals, too-many-public-methods, # too-many-statements, too-many-instance-attributes, too-many-branches @@ -275,6 +276,9 @@ from build123d.build_enums import ( Until, ) from build123d.geometry import ( + DEG2RAD, + TOLERANCE, + Axis, BoundBox, Color, @@ -283,20 +287,13 @@ from build123d.geometry import ( Plane, Vector, VectorLike, + + logger, ) -# Create a build123d logger to distinguish these logs from application logs. -# If the user doesn't configure logging, all build123d logs will be discarded. -logging.getLogger("build123d").addHandler(logging.NullHandler()) -logger = logging.getLogger("build123d") -TOLERANCE = 1e-6 -TOL = 1e-2 -DEG2RAD = pi / 180.0 -RAD2DEG = 180 / pi HASH_CODE_MAX = 2147483647 # max 32bit signed int, required by OCC.Core.HashCode - shape_LUT = { ta.TopAbs_VERTEX: "Vertex", ta.TopAbs_EDGE: "Edge", @@ -4038,18 +4035,8 @@ class Compound(Mixin3D, Shape): # Align the text from the bounding box align = tuplify(align, 2) - bbox = text_flat.bounding_box() - align_offset = [] - for i in range(2): - if align[i] == Align.MIN: - align_offset.append(-bbox.min.to_tuple()[i]) - elif align[i] == Align.CENTER: - align_offset.append( - -(bbox.min.to_tuple()[i] + bbox.max.to_tuple()[i]) / 2 - ) - elif align[i] == Align.MAX: - align_offset.append(-bbox.max.to_tuple()[i]) - text_flat = text_flat.translate(Vector(*align_offset)) + text_flat = text_flat.translate(Vector( + *text_flat.bounding_box().to_align_offset(align))) if text_path is not None: path_length = text_path.length diff --git a/src/build123d/version.py b/src/build123d/version.py index bd63569..1bca9df 100644 --- a/src/build123d/version.py +++ b/src/build123d/version.py @@ -1,15 +1,18 @@ +""" +Export a version string. +""" try: try: - from ._dev.scm_version import version + from ._dev.scm_version import version # pylint: disable=unused-import except ImportError: from ._version import version -except Exception: +except Exception: # pylint: disable=broad-exception-caught import warnings warnings.warn( - f'could not determine {__name__.split(".")[0]} package version; ' + f'could not determine {__name__.split(".", maxsplit=1)[0]} package version; ' "this indicates a broken installation" ) del warnings - version = "0.0.0" \ No newline at end of file + version = "0.0.0" # pylint: disable=invalid-name