Skip to content
This repository has been archived by the owner on Feb 29, 2024. It is now read-only.

Fix more PEP 8 findings and warnings #723

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
522 changes: 276 additions & 246 deletions labelImg.py

Large diffs are not rendered by default.

22 changes: 11 additions & 11 deletions libs/canvas.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def leaveEvent(self, ev):
def focusOutEvent(self, ev):
self.restore_cursor()

def isVisible(self, shape):
def is_visible(self, shape):
return self.visible.get(shape, True)

def drawing(self):
Expand Down Expand Up @@ -135,7 +135,7 @@ def mouseMoveEvent(self, ev):
clipped_y = min(max(0, pos.y()), size.height())
pos = QPointF(clipped_x, clipped_y)
elif len(self.current) > 1 and self.close_enough(pos, self.current[0]):
# Attract line to starting point and colorise to alert the
# Attract line to starting point and colorize to alert the
# user:
pos = self.current[0]
color = self.current.line_color
Expand Down Expand Up @@ -197,7 +197,7 @@ def mouseMoveEvent(self, ev):
# - Highlight vertex
# Update shape/vertex fill and tooltip value accordingly.
self.setToolTip("Image")
for shape in reversed([s for s in self.shapes if self.isVisible(s)]):
for shape in reversed([s for s in self.shapes if self.is_visible(s)]):
# Look for a nearby vertex to highlight. If that fails,
# check if we happen to be inside a shape.
index = shape.nearest_vertex(pos, self.epsilon)
Expand Down Expand Up @@ -342,7 +342,7 @@ def select_shape_point(self, point):
self.select_shape(shape)
return self.h_vertex
for shape in reversed(self.shapes):
if self.isVisible(shape) and shape.contains_point(point):
if self.is_visible(shape) and shape.contains_point(point):
self.select_shape(shape)
self.calculate_offsets(shape, point)
return self.selected_shape
Expand Down Expand Up @@ -395,8 +395,6 @@ def bounded_move_vertex(self, pos):

left_index = (index + 1) % 4
right_index = (index + 3) % 4
left_shift = None
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rationale: Variables are overwritten in all following paths, so they can be removed

right_shift = None
if index % 2 == 0:
right_shift = QPointF(shift_pos.x(), 0)
left_shift = QPointF(0, shift_pos.y())
Expand Down Expand Up @@ -481,7 +479,7 @@ def paintEvent(self, event):
Shape.scale = self.scale
Shape.label_font_size = self.label_font_size
for shape in self.shapes:
if (shape.selected or not self._hide_background) and self.isVisible(shape):
if (shape.selected or not self._hide_background) and self.is_visible(shape):
shape.fill = shape.selected or shape == self.h_shape
shape.paint(p)
if self.current:
Expand All @@ -503,8 +501,8 @@ def paintEvent(self, event):

if self.drawing() and not self.prev_point.isNull() and not self.out_of_pixmap(self.prev_point):
p.setPen(QColor(0, 0, 0))
p.drawLine(self.prev_point.x(), 0, self.prev_point.x(), self.pixmap.height())
p.drawLine(0, self.prev_point.y(), self.pixmap.width(), self.prev_point.y())
p.drawLine(int(self.prev_point.x()), 0, int(self.prev_point.x()), int(self.pixmap.height()))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rationale: There was an implicit conversion from float to int.

I changed it so that this is now explicit.

Both the implicit and the explicit conversions round down. If you want to have proper rounding up at 0.5, this needs to be changed.

To do:

  • Review and change if necessary

p.drawLine(0, int(self.prev_point.y()), int(self.pixmap.width()), int(self.prev_point.y()))

self.setAutoFillBackground(True)
if self.verified:
Expand Down Expand Up @@ -681,7 +679,8 @@ def set_shape_visible(self, shape, value):
self.visible[shape] = value
self.repaint()

def current_cursor(self):
@staticmethod
def current_cursor():
cursor = QApplication.overrideCursor()
if cursor is not None:
cursor = cursor.shape()
Expand All @@ -694,7 +693,8 @@ def override_cursor(self, cursor):
else:
QApplication.changeOverrideCursor(cursor)

def restore_cursor(self):
@staticmethod
def restore_cursor():
QApplication.restoreOverrideCursor()

def reset_state(self):
Expand Down
2 changes: 1 addition & 1 deletion libs/colorDialog.py → libs/color_dialog.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def __init__(self, parent=None):
self.bb.addButton(BB.RestoreDefaults)
self.bb.clicked.connect(self.check_restore)

def getColor(self, value=None, title=None, default=None):
def get_color(self, value=None, title=None, default=None):
self.default = default
if title:
self.setWindowTitle(title)
Expand Down
5 changes: 4 additions & 1 deletion libs/combobox.py → libs/combo_box.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,12 @@


class ComboBox(QWidget):
def __init__(self, parent=None, items=[]):
def __init__(self, parent=None, items=None):
Copy link
Contributor Author

@Cerno-b Cerno-b Mar 22, 2021

Choose a reason for hiding this comment

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

Rationale: Function arguments should not be mutable

super(ComboBox, self).__init__(parent)

if items is None:
items = []

layout = QHBoxLayout()
self.cb = QComboBox()
self.items = items
Expand Down
6 changes: 3 additions & 3 deletions libs/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
SETTING_AUTO_SAVE = 'autosave'
SETTING_SINGLE_CLASS = 'singleclass'
FORMAT_PASCALVOC='PascalVOC'
FORMAT_YOLO='YOLO'
FORMAT_CREATEML='CreateML'
FORMAT_YOLO = 'YOLO'
FORMAT_CREATEML = 'CreateML'
SETTING_DRAW_SQUARE = 'draw/square'
SETTING_LABEL_FILE_FORMAT= 'labelFileFormat'
SETTING_LABEL_FILE_FORMAT = 'labelFileFormat'
DEFAULT_ENCODING = 'utf-8'
3 changes: 2 additions & 1 deletion libs/create_ml_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ def write(self):

Path(self.output_file).write_text(json.dumps(output_dict), ENCODE_METHOD)

def calculate_coordinates(self, x1, x2, y1, y2):
@staticmethod
def calculate_coordinates(x1, x2, y1, y2):
if x1 < x2:
x_min = x1
x_max = x2
Expand Down
File renamed without changes.
13 changes: 4 additions & 9 deletions libs/labelFile.py → libs/label_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,13 @@
except ImportError:
from PyQt4.QtGui import QImage

from base64 import b64encode, b64decode
from libs.pascal_voc_io import PascalVocWriter
from libs.yolo_io import YOLOWriter
from libs.pascal_voc_io import XML_EXT
from libs.create_ml_io import CreateMLWriter
from libs.create_ml_io import JSON_EXT
from enum import Enum
import os.path
import sys


class LabelFileFormat(Enum):
Expand All @@ -32,13 +30,13 @@ class LabelFile(object):
# suffix = '.lif'
suffix = XML_EXT

def __init__(self, filename=None):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rationale: Removed unused function argument

def __init__(self):
self.shapes = ()
self.image_path = None
self.image_data = None
self.verified = False

def save_create_ml_format(self, filename, shapes, image_path, image_data, class_list, line_color=None, fill_color=None, database_src=None):
Copy link
Contributor Author

@Cerno-b Cerno-b Mar 22, 2021

Choose a reason for hiding this comment

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

Rationale: Removed unused function arguments (also for yolo and pascal formats)

This might be a preparation for new functionality or an artifact from old code. Since you can choose box colors, but are unable to save them, I assume it's from old code and I think it makes sense to decide to either drop the box color support althogether or re-introduce saving the colors to the output files.

Following the YAGNI principle, I removed this. If you want to keep it, I can revert the change. I would suggest adding an issue about saving box colors in that case so it is not forgotten.

def save_create_ml_format(self, filename, shapes, image_path):
img_folder_path = os.path.dirname(image_path)
img_folder_name = os.path.split(img_folder_path)[-1]
img_file_name = os.path.basename(image_path)
Expand All @@ -54,9 +52,7 @@ def save_create_ml_format(self, filename, shapes, image_path, image_data, class_
writer.verified = self.verified
writer.write()


def save_pascal_voc_format(self, filename, shapes, image_path, image_data,
line_color=None, fill_color=None, database_src=None):
def save_pascal_voc_format(self, filename, shapes, image_path, image_data):
img_folder_path = os.path.dirname(image_path)
img_folder_name = os.path.split(img_folder_path)[-1]
img_file_name = os.path.basename(image_path)
Expand Down Expand Up @@ -85,8 +81,7 @@ def save_pascal_voc_format(self, filename, shapes, image_path, image_data,
writer.save(target_file=filename)
return

def save_yolo_format(self, filename, shapes, image_path, image_data, class_list,
line_color=None, fill_color=None, database_src=None):
def save_yolo_format(self, filename, shapes, image_path, image_data, class_list):
img_folder_path = os.path.dirname(image_path)
img_folder_name = os.path.split(img_folder_path)[-1]
img_file_name = os.path.basename(image_path)
Expand Down
23 changes: 13 additions & 10 deletions libs/pascal_voc_io.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#!/usr/bin/env python
# -*- coding: utf8 -*-
import sys
from xml.etree import ElementTree
from xml.etree.ElementTree import Element, SubElement
from lxml import etree
Expand All @@ -12,6 +11,7 @@
XML_EXT = '.xml'
ENCODE_METHOD = DEFAULT_ENCODING


class PascalVocWriter:

def __init__(self, folder_name, filename, img_size, database_src='Unknown', local_img_path=None):
Expand All @@ -23,16 +23,17 @@ def __init__(self, folder_name, filename, img_size, database_src='Unknown', loca
self.local_img_path = local_img_path
self.verified = False

def prettify(self, elem):
@staticmethod
def prettify(elem):
"""
Return a pretty-printed XML string for the Element.
"""
rough_string = ElementTree.tostring(elem, 'utf8')
root = etree.fromstring(rough_string)
return etree.tostring(root, pretty_print=True, encoding=ENCODE_METHOD).replace(" ".encode(), "\t".encode())
# minidom does not support UTF-8
# reparsed = minidom.parseString(rough_string)
# return reparsed.toprettyxml(indent="\t", encoding=ENCODE_METHOD)
# re_parsed = minidom.parseString(rough_string)
# return re_parsed.toprettyxml(indent="\t", encoding=ENCODE_METHOD)

def gen_xml(self):
"""
Expand Down Expand Up @@ -78,9 +79,13 @@ def gen_xml(self):
return top

def add_bnd_box(self, x_min, y_min, x_max, y_max, name, difficult):
bnd_box = {'xmin': x_min, 'ymin': y_min, 'xmax': x_max, 'ymax': y_max}
Copy link
Contributor Author

@Cerno-b Cerno-b Mar 22, 2021

Choose a reason for hiding this comment

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

Rationale: Unified the definition into a single dictionary creation call

bnd_box['name'] = name
bnd_box['difficult'] = difficult
bnd_box = {
'xmin': x_min,
'ymin': y_min,
'xmax': x_max,
'ymax': y_max,
'name': name,
'difficult': difficult}
self.box_list.append(bnd_box)

def append_objects(self, top):
Expand Down Expand Up @@ -112,7 +117,6 @@ def append_objects(self, top):
def save(self, target_file=None):
root = self.gen_xml()
self.append_objects(root)
out_file = None
Copy link
Contributor Author

@Cerno-b Cerno-b Mar 22, 2021

Choose a reason for hiding this comment

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

Rationale: Variable is overwritten in all paths, so it can be removed

if target_file is None:
out_file = codecs.open(
self.filename + XML_EXT, 'w', encoding=ENCODE_METHOD)
Expand All @@ -128,7 +132,7 @@ class PascalVocReader:

def __init__(self, file_path):
# shapes type:
# [labbel, [(x1,y1), (x2,y2), (x3,y3), (x4,y4)], color, color, difficult]
# [label, [(x1,y1), (x2,y2), (x3,y3), (x4,y4)], color, color, difficult]
self.shapes = []
self.file_path = file_path
self.verified = False
Expand All @@ -152,7 +156,6 @@ def parse_xml(self):
assert self.file_path.endswith(XML_EXT), "Unsupported file format"
parser = etree.XMLParser(encoding=ENCODE_METHOD)
xml_tree = ElementTree.parse(self.file_path, parser=parser).getroot()
filename = xml_tree.find('filename').text
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rationale: Variable is never used. Not sure whether this is a code artifact or points to a bug. May make sense to look into it

To do:

  • check if this is correct behavior

try:
verified = xml_tree.attrib['verified']
if verified == 'yes':
Expand Down
1 change: 0 additions & 1 deletion libs/settings.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import pickle
import os
import sys


class Settings(object):
Expand Down
17 changes: 9 additions & 8 deletions libs/stringBundle.py → libs/string_bundle.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ def get_string(self, string_id):
assert(string_id in self.id_to_message), "Missing string id : " + string_id
return self.id_to_message[string_id]

def __create_lookup_fallback_list(self, locale_str):
@staticmethod
def __create_lookup_fallback_list(locale_str):
result_paths = []
base_path = ":/strings"
result_paths.append(base_path)
Expand All @@ -56,18 +57,18 @@ def __create_lookup_fallback_list(self, locale_str):
return result_paths

def __load_bundle(self, path):
PROP_SEPERATOR = '='
prop_seperator = '='
f = QFile(path)
if f.exists():
if f.open(QIODevice.ReadOnly | QFile.Text):
text = QTextStream(f)
text.setCodec("UTF-8")

while not text.atEnd():
line = ustr(text.readLine())
key_value = line.split(PROP_SEPERATOR)
key = key_value[0].strip()
value = PROP_SEPERATOR.join(key_value[1:]).strip().strip('"')
self.id_to_message[key] = value
while not text.atEnd():
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rationale: This would have lead to a crash if f.open() failed.

line = ustr(text.readLine())
key_value = line.split(prop_seperator)
key = key_value[0].strip()
value = prop_seperator.join(key_value[1:]).strip().strip('"')
self.id_to_message[key] = value

f.close()
File renamed without changes.
1 change: 1 addition & 0 deletions libs/ustr.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import sys
from libs.constants import DEFAULT_ENCODING


def ustr(x):
"""py2/py3 unicode helper"""

Expand Down
23 changes: 10 additions & 13 deletions libs/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,6 @@ def label_validator():
return QRegExpValidator(QRegExp(r'^[^ \t].+'), None)


class Struct(object):
Copy link
Contributor Author

Choose a reason for hiding this comment

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


def __init__(self, **kwargs):
self.__dict__.update(kwargs)


def distance(p):
return sqrt(p.x() * p.x() + p.y() * p.y())

Expand All @@ -85,19 +79,22 @@ def generate_color_by_text(text):
b = int((hash_code / 16581375) % 255)
return QColor(r, g, b, 100)


def have_qstring():
"""p3/qt5 get rid of QString wrapper as py3 has native unicode str type"""
return not (sys.version_info.major >= 3 or QT_VERSION_STR.startswith('5.'))

def util_qt_strlistclass():
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rationale: Remove unused function

return QStringList if have_qstring() else list

def natural_sort(list, key=lambda s:s):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rationale: Avoid using reserved names in variables

def natural_sort(input_list, key=lambda s: s):
"""
Sort the list into natural alphanumeric order.
"""
def get_alphanum_key_func(key):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rationale: Avoid using reserved names in variables

convert = lambda text: int(text) if text.isdigit() else text
return lambda s: [convert(c) for c in re.split('([0-9]+)', key(s))]
def get_alphanum_key_func(internal_key):

def convert(text):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rationale: Avoid assigning a lambda expression, prefer using a function instead

return int(text) if text.isdigit() else text

return lambda s: [convert(c) for c in re.split('([0-9]+)', internal_key(s))]

sort_key = get_alphanum_key_func(key)
list.sort(key=sort_key)
input_list.sort(key=sort_key)
Loading