В какой-то момент времени я превратился в педанта брюзгу. В фильмах малейшие нестыковки и провалы в логике портят мне весь просмотр. В чатах меня бесит it's вместо its. А в статьях про программирование... Всё плохо. За меня всё уже сказал @AlexanderAstafiev, я лишь процитирую:
Простите, я не могу так больше. Я слишком хорошо знаю Python, чтобы молчать при виде такого кода.
Я устал. Я не могу это читать. Простите за токсичную критику, накипело.
Самое забавное, что, по моим ощущениям, везде я вижу одни и те же классы проблем. Я даже запилил сервис, где можно закинуть код и получить код ревью, и, собрав немного статистики, понял, что 50 типов ошибок достаточно, чтобы покрыть большую часть проблем в чужом коде. Но выборка у меня была небольшая, и я подумал: а что, если проверить много кода? И всё заверте...
Давайте вспомним про гругов
Где-то тут проскакивала замечательная статья про гругов: на русском, на английском.
разработчик с мозгом груга не очень умный, но разработчик с мозгом груга программирует много лет и научился кое чему, хоть всё равно часто запутывается
Я как прочитал её, так сразу и понял: это я. Не очень умный программист, но за плечами горы опыта, и это формирует мой подход к написанию кода: не усложняй. На работе у нас есть чел - большая голова, он держит весь проект в своей "оперативной памяти" и пишет просто полотна кода с кучей ветвлений, и где-то в конце он ещё помнит, что в начале был if и нужно рассмотреть else. Я так не могу, я когда прыгаю jump-to-definition, на четвёртом уровне вложенности уже забываю, откуда я пришёл и что я делал. Я как бельчонок с хорошеньким хвостиком, если вы понимаете, о чём я.
Что же касается бельчонка, то он сидел в лесной чаще и рассеянно поглядывал то на одно дерево, то на другое. Он не мог, даже если бы пришлось пожертвовать своим хвостиком, вспомнить, в дупле какого дерева он жил и вообще ради чего он прискакал в лес и что там искал.
Я не могу писать большие и сложные штуки, а если и напишу, то там же и умру. Поэтому философия гругов мне очень подходит:
главный охотник на груга это сложность
сложность плохо
скажу снова:
сложность очень плохо
теперь ты скажи сам:
сложность очень, очень плохо
если выбрать между сложность и биться с динозавром, груг выбери динозавр: груг хотя бы видеть динозавра
И в этой статье я хотел немного поделиться тем, как можно бороться со сложностью в питоне.
Разработчик с мозгом груга хочет собрать знания в маленькую, простую и интересную страницу
Если вы умный
... то всё равно нужно стремиться упрощать. Потому что если вы пишете статью или вкладываете что-то в опенсорс, ваш код (внезапно) читают и/или запускают. Давайте заботиться о пользователях вашего кода? Кстати, пользователем вашего когда можете стать вы сами через несколько лет.
Что значит "упрощать" и "заботиться о пользователях"?
Я понимаю это как написание кода, который
легко читать
легко изменять
легко отлаживать
легко тестировать
Как писать такой код? Использовать линтеры и мозг, чтобы уменьшать сложность. Давайте обсудим типичные ошибки и антипаттерны.
Ты не сможешь рассмотреть все возможные ошибки
Действительно, выстрелить себе в ногу можно миллионом способов, особенно в динамически типизированном языке. Но мне поможет мой хоуми aka Николай Гумилёв Вильфредо Парето.
Закон Парето
В большинстве случаев около 80% результата происходят из 20% причин.
А вдруг мы можем рассмотреть топ 20% ошибок и надеяться, что покроем 80% случаев?
А как ты узнаешь самые распространённые ошибки?
Автоматически.
Вообще не всё можно отловить программно. Например, плохой дизайн классов и модулей. Как-то я видел что-то вроде:
def swagger_support(func):
@wraps(func)
def wrapper(self, *args, **kwargs):
if args[0] == 1:
return getattr(super(type(self), self), func.__name__)(*args, **kwargs)
else:
return func(self, *args, **kwargs)
return wrapper
Не знаю, какой там был контекст, но если в зависимости от какого-то параметра вызывается либо родительский метод, либо метод наследника, то, возможно, где-то на уровне выше нужно было в зависимости от параметра передать либо объект родителя, либо объект наследника - наследование так и работает, оно переопределяет метод, вместо того чтобы заставлять вас писать if. Как подобное отлавливать - я не знаю. Это самый интересный класс ошибок, но в этой статье их не будет, сорян.
Итак, как автоматически искать ошибки? Для этого я построю вундервафлю, прям как аннигилятор в "Полицейский из Беверли-Хиллз 3".
Если эта штука напоминает вам flake8 с плагинами, то вы угадали - это он и есть.
Почему не pylint | mypy | dlint | black | ...? Мне нужен был линтер (не форматтер), в котором есть максимальное число подходящих мне плагинов, и для которого я мог бы легко написать свои. Flake8 мне показался наиболее подходящим.
Итак, я прошёлся по awesome flake8, поскрёб по закоулкам гитхаба, написал свой мини-плагин с тем, чего не хватало, и получилось вот это:
flake8==5.0.4
flake8-json==21.7.0
flake8-adjustable-complexity==0.0.6
flake8-annotations-coverage==0.0.6
flake8-annotations==2.9.1
flake8-builtins==1.5.3
flake8-cognitive-complexity==0.1.0
flake8-commas==2.1.0
flake8-comprehensions==3.10.0
flake8-eradicate==1.3.0
flake8-expression-complexity==0.0.11
flake8-functions==0.0.7
flake8-simplify==0.19.3
flake8-scream==0.1.0
git+https://github.com/i02sopop/flake8-global-variables
pep8-naming==0.13.1
flake8-bugbear==22.7.1
flake8-pie==0.16.0
flake8-print==5.0.0
flake8-use-pathlib==0.3.0
git+https://github.com/c0ntribut0r/flake8-grug
pylint==2.14.5
flake8-pylint==0.1.3
Я постарался отобрать те плагины, которые сделают код чище, но не стал включать спорные штуки (например, всякие следилки за докстрингами) и базовые и очевидные вещи (форматирование, всякие там отступы и пробелы). Даже среди того, что я выбрал, было много мусора, поэтому я отбирал ошибки поштучно, если интересно - вот мой .flake8, он человекочитаем:
.flake8
[flake8]
# statistics = True
select =
# -------- https://github.com/best-doctor/flake8-adjustable-complexity --------
CAC001,
# func is too complex (complexity > max allowed complexity)
CAC002,
# func is too complex (complexity). Bad variable names penalty is too high (penalty)
# -------- https://github.com/best-doctor/flake8-annotations-coverage --------
TAE001,
# Too few type annotations in file
# -------- https://github.com/sco1/flake8-annotations --------
ANN001,
# Missing type annotation for function argument
ANN002,
# Missing type annotation for *args
ANN003,
# Missing type annotation for **kwargs
; ANN101,
# Missing type annotation for self in method
; ANN102,
# Missing type annotation for cls in classmethod
ANN201,
# Missing return type annotation for public function
ANN202,
# Missing return type annotation for protected function
ANN203,
# Missing return type annotation for secret function
ANN204,
# Missing return type annotation for special method
ANN205,
# Missing return type annotation for staticmethod
ANN206,
# Missing return type annotation for classmethod
ANN301,
# PEP 484 disallows both type annotations and type comments
ANN401,
# Dynamically typed expressions (typing.Any) are disallowed.2
# -------- https://github.com/gforcada/flake8-builtins --------
A001,
# variable "max" is shadowing a python builtin
A002,
# argument "dict" is shadowing a python builtin
# -------- https://github.com/Melevir/flake8-cognitive-complexity --------
CCR001,
# Cognitive complexity is too high (X > Y)
# -------- https://github.com/PyCQA/flake8-commas --------
# C812, # too many FPs
# missing trailing comma
C813,
# missing trailing comma in Python 3
# C814,
# missing trailing comma in Python 2
C815,
# missing trailing comma in Python 3.5+
C816,
# missing trailing comma in Python 3.6+
C818,
# trailing comma on bare tuple prohibited
C819,
# trailing comma prohibited
# -------- https://github.com/adamchainz/flake8-comprehensions --------
C400,
C401,
C402,
# Unnecessary generator - rewrite as a <list/set/dict> comprehension.
C403,
C404,
# Unnecessary list comprehension - rewrite as a <set/dict> comprehension.
C405,
C406,
# Unnecessary <list/tuple> literal - rewrite as a <set/dict> literal.
# C408,
# Unnecessary <dict/list/tuple> call - rewrite as a literal.
C409,
C410,
# Unnecessary <list/tuple> passed to <list/tuple>() - (remove the outer call to <list/tuple>``()/rewrite as a ``<list/tuple> literal).
C411,
# Unnecessary list call - remove the outer call to list().
C413,
# Unnecessary <list/reversed> call around sorted().
C414,
# Unnecessary <list/reversed/set/sorted/tuple> call within <list/set/sorted/tuple>().
C415,
# Unnecessary subscript reversal of iterable within <reversed/set/sorted>().
C416,
# Unnecessary <list/set> comprehension - rewrite using <list/set>().
C417,
# Unnecessary map usage - rewrite using a generator expression/<list/set/dict> comprehension.
# -------- https://github.com/wemake-services/flake8-eradicate --------
# E800,
# Found commented out code
# -------- https://github.com/best-doctor/flake8-expression-complexity --------
ECE001,
# Expression is too complex (X > Y)
# -------- https://github.com/best-doctor/flake8-functions --------
CFQ001,
# function length (default max length is 100)
# CFQ002,
# function arguments number (default max arguments amount is 6)
CFQ003,
# function is not pure.
; CFQ004,
# function returns number (default max returns amount is 3)
# -------- https://github.com/MartinThoma/flake8-simplify --------
# Python-specific rules:
SIM101,
# Multiple isinstance-calls which can be merged into a single call by using a tuple as a second argument (example)
SIM104,
#: Use 'yield from iterable' (introduced in Python 3.3, see PEP 380) (example)
SIM105,
# Use 'contextlib.suppress(...)' instead of try-except-pass (example)
SIM107,
# Don't use return in try/except and finally (example)
SIM108,
# Use the ternary operator if it's reasonable (example)
SIM109,
# Use a tuple to compare a value against multiple values (example)
SIM110,
# Use any(...) (example)
SIM111,
# Use all(...) (example)
SIM113,
# Use enumerate instead of manually incrementing a counter (example)
SIM114,
# Combine conditions via a logical or to prevent duplicating code (example)
SIM115,
# Use context handler for opening files (example)
SIM116,
# Use a dictionary instead of many if/else equality checks (example)
SIM117,
# Merge with-statements that use the same scope (example)
SIM119,
# Moved to flake8-scream due to issue 63
SIM120,
#: Use 'class FooBar:' instead of 'class FooBar(object):' (example)
SIM121,
# Reserved for SIM908 once it's stable
SIM125,
# Reserved for SIM905 once it's stable
SIM126,
# Reserved for SIM906 once it's stable
SIM127,
# Reserved for SIM907 once it's stable
# Simplifying Comparisons:
SIM201,
# Use 'a != b' instead of 'not a == b' (example)
SIM202,
# Use 'a == b' instead of 'not a != b' (example)
SIM203,
# Use 'a not in b' instead of 'not (a in b)' (example)
SIM204,
# Moved to flake8-scream due to issue 116
SIM205,
# Moved to flake8-scream due to issue 116
SIM206,
# Moved to flake8-scream due to issue 116
SIM207,
# Moved to flake8-scream due to issue 116
SIM208,
# Use 'a' instead of 'not (not a)' (example)
SIM210,
# Use 'bool(a)' instead of 'True if a else False' (example)
SIM211,
# Use 'not a' instead of 'False if a else True' (example)
SIM212,
# Use 'a if a else b' instead of 'b if not a else a' (example)
SIM220,
# Use 'False' instead of 'a and not a' (example)
SIM221,
# Use 'True' instead of 'a or not a' (example)
SIM222,
# Use 'True' instead of '... or True' (example)
SIM223,
# Use 'False' instead of '... and False' (example)
SIM224,
# Reserved for SIM901 once it's stable
SIM300,
# Use 'age == 42' instead of '42 == age' (example)
# Simplifying usage of dictionaries:
SIM401,
# Use 'a_dict.get(key, "default_value")' instead of an if-block (example)
SIM118,
# Use 'key in dict' instead of 'key in dict.keys()' (example)
# General Code Style:
SIM102,
# Use a single if-statement instead of nested if-statements (example)
SIM103,
# Return the boolean condition directly (example)
SIM106,
# Handle error-cases first (example). This rule was removed due to too many false-positives.
SIM112,
# Use CAPITAL environment variables (example)
SIM122,
#/ SIM902: Moved to flake8-scream due to issue 125
SIM123,
#/ SIM902: Moved to flake8-scream due to issue 130
SIM124,
# Reserved for SIM909 once it's stable
# Current experimental rules:
SIM901,
# Use comparisons directly instead of wrapping them in a bool(...) call (example)
SIM904,
# Assign values to dictionary directly at initialization (example)
SIM905,
# Split string directly if only constants are used (example)
SIM906,
# Merge nested os.path.join calls (example)
SIM907,
# Use Optional[Type] instead of Union[Type, None] (example)
SIM908,
# Use dict.get(key) (example)
SIM909,
# Avoid reflexive assignments (example)
# -------- https://github.com/MartinThoma/flake8-scream --------
SCR119,
#: Use dataclasses for data containers (example)
SCR902,
# Use keyword-argument instead of magic boolean (example)
SCR903,
# Use keyword-argument instead of magic number (example)
# -------- https://github.com/i02sopop/flake8-global-variables --------
# W001,
# Global variable {0} defined
W002,
# Global variable {0} used
# ---- https://github.com/PyCQA/pep8-naming ----
# N801,
# class names should use CapWords convention (class names)
# N802,
# function name should be lowercase (function names)
# N803,
# argument name should be lowercase (function arguments)
N804,
# first argument of a classmethod should be named 'cls' (function arguments)
N805,
# first argument of a method should be named 'self' (function arguments)
N806,
# variable in function should be lowercase
N807,
# function name should not start and end with '__'
N811,
# constant imported as non constant (constants)
N812,
# lowercase imported as non-lowercase
N813,
# camelcase imported as lowercase
N814,
# camelcase imported as constant (distinct from N817 for selective enforcement)
N815,
# mixedCase variable in class scope (constants, method names)
N816,
# mixedCase variable in global scope (constants)
N817,
# camelcase imported as acronym (distinct from N814 for selective enforcement)
N818,
# error suffix in exception names (exceptions)
# -------- https://github.com/PyCQA/flake8-bugbear --------
B001,
# Do not use bare except:, it also catches unexpected events like memory errors, interrupts, system exit, and so on. Prefer except Exception:. If you're sure what you're doing, be explicit and write except BaseException:. Disable E722 to avoid duplicate warnings.
B002,
# Python does not support the unary prefix increment. Writing ++n is equivalent to +(+(n)), which equals n. You meant n += 1.
B003,
# Assigning to os.environ doesn't clear the environment. Subprocesses are going to see outdated variables, in disagreement with the current process. Use os.environ.clear() or the env= argument to Popen.
B004,
# Using hasattr(x, '__call__') to test if x is callable is unreliable. If x implements custom __getattr__ or its __call__ is itself not callable, you might get misleading results. Use callable(x) for consistent results.
B005,
# Using .strip() with multi-character strings is misleading the reader. It looks like stripping a substring. Move your character set to a constant if this is deliberate. Use .replace() or regular expressions to remove string fragments.
B006,
# Do not use mutable data structures for argument defaults. They are created during function definition time. All calls to the function reuse this one instance of that data structure, persisting changes between them.
B007,
# Loop control variable not used within the loop body. If this is intended, start the name with an underscore.
B008,
# Do not perform function calls in argument defaults. The call is performed only once at function definition time. All calls to your function will reuse the result of that definition-time function call. If this is intended, assign the function call to a module-level variable and use that variable as a default value.
B009,
# Do not call getattr(x, 'attr'), instead use normal property access: x.attr. Missing a default to getattr will cause an AttributeError to be raised for non-existent properties. There is no additional safety in using getattr if you know the attribute name ahead of time.
B010,
# Do not call setattr(x, 'attr', val), instead use normal property access: x.attr = val. There is no additional safety in using setattr if you know the attribute name ahead of time.
B011,
# Do not call assert False since python -O removes these calls. Instead callers should raise AssertionError().
B012,
# Use of break, continue or return inside finally blocks will silence exceptions or override return values from the try or except blocks. To silence an exception, do it explicitly in the except block. To properly use a break, continue or return refactor your code so these statements are not in the finally block.
B013,
# A length-one tuple literal is redundant. Write except SomeError: instead of except (SomeError,):.
B014,
# Redundant exception types in except (Exception, TypeError):. Write except Exception:, which catches exactly the same exceptions.
B015,
# Pointless comparison. This comparison does nothing but waste CPU instructions. Either prepend assert or remove it.
B016,
# Cannot raise a literal. Did you intend to return it or raise an Exception?
B017,
# self.assertRaises(Exception): should be considered evil. It can lead to your test passing even if the code being tested is never executed due to a typo. Either assert for a more specific exception (builtin or custom), use assertRaisesRegex, or use the context manager form of assertRaises (with self.assertRaises(Exception) as ex:) with an assertion against the data available in ex.
B018,
# Found useless expression. Either assign it to a variable or remove it.
B019,
# Use of functools.lru_cache or functools.cache on methods can lead to memory leaks. The cache may retain instance references, preventing garbage collection.
B020,
# Loop control variable overrides iterable it iterates
B021,
# f-string used as docstring. This will be interpreted by python as a joined string rather than a docstring.
B022,
# No arguments passed to contextlib.suppress. No exceptions will be suppressed and therefore this context manager is redundant. N.B. this rule currently does not flag suppress calls to avoid potential false positives due to similarly named user-defined functions.
B023,
# Functions defined inside a loop must not use variables redefined in the loop, because late-binding closures are a classic gotcha.
# -------- https://github.com/sbdchd/flake8-pie --------
; PIE781,
# assign-and-return
# Based on Clippy's let_and_return and Microsoft's TSLint rule no-unnecessary-local-variable.
# PIE783,
# celery-explicit-names
# Warn about Celery task definitions that don't have explicit names.
# PIE784,
# celery-explicit-crontab-args
# The crontab class provided by Celery has some default args that are suprising to new users. Specifically, crontab(hour="0,12") won't run a task at midnight and noon, it will run the task at every minute during those two hours. This lint makes that call an error, forcing you to write crontab(hour="0, 12", minute="*").
# PIE785,
# celery-require-tasks-expire
# Celery tasks can bunch up if they don't have expirations.
PIE786,
# precise-exception-handlers
# Be precise in what exceptions you catch. Bare except: handlers, catching BaseException, or catching Exception can lead to unexpected bugs.
PIE787,
# no-len-condition
# Empty collections are fasley in Python so calling len() is unnecessary when checking for emptiness in an if statement/expression.
PIE788,
# no-bool-condition
# If statements/expressions evalute the truthiness of the their test argument, so calling bool() is unnecessary.
PIE789,
# prefer-isinstance-type-compare
# Using type() doesn't take into account subclassess and type checkers won't refine the type, use isinstance instead.
PIE790,
# no-unnecessary-pass
# pass is unnecessary when definining a class or function with an empty body.
PIE791,
# no-pointless-statements
# Comparisions without an assignment or assertion are probably a typo.
# PIE792, # same as SIM120
# no-inherit-object
# Inheriting from object isn't necessary in Python 3.
PIE793,
# prefer-dataclass
# Attempts to find cases where the @dataclass decorator is unintentionally missing.
PIE794,
# dupe-class-field-definitions
# Finds duplicate definitions for the same field, which can occur in large ORM model definitions.
PIE795,
# prefer-stdlib-enums
# Instead of defining various constant properties on a class, use the stdlib enum which typecheckers support for type refinement.
PIE796,
# prefer-unique-enums
# By default the stdlib enum allows multiple field names to map to the same value, this lint requires each enum value be unique.
PIE797,
# no-unnecessary-if-expr
# Call bool() directly rather than reimplementing its functionality.
PIE798,
# no-unnecessary-class
# Instead of using class to namespace functions, use a module.
PIE799,
# prefer-col-init
# Check that values are passed in when collections are created rather than creating an empty collection and then inserting.
PIE800,
# no-unnecessary-spread
# Check for unnecessary dict unpacking.
PIE801,
# prefer-simple-return
# Return boolean expressions directly instead of returning True and False.
PIE802,
# prefer-simple-any-all
# Remove unnecessary comprehensions for any and all
PIE803,
# prefer-logging-interpolation
# Don't format strings before logging. Let logging interpolate arguments.
PIE804,
# no-unnecessary-dict-kwargs
# As long as the keys of the dict are valid Python identifier names, we can safely remove the surrounding dict.
# PIE805,
# prefer-literal
# Currently only checks for byte string literals.
PIE806,
# no-assert-except
# Instead of asserting and catching the exception, use an if statement.
PIE807,
# prefer-list-builtin
# lambda: [] is equivalent to the builtin list
PIE808,
# prefer-simple-range
# We can leave out the first argument to range in some cases since the default start position is 0.
PIE809,
# django-prefer-bulk
# Bulk create multiple objects instead of executing O(N) queries.
# -------- https://github.com/JBKahn/flake8-print --------
T201,
# print found
T203,
# pprint found
T204,
# pprint declared
# -------- https://gitlab.com/RoPP/flake8-use-pathlib --------
PL100,
# os.path.abspath("foo") should be replaced by foo_path.resolve()
PL101,
# os.chmod("foo", 0o444) should be replaced by foo_path.chmod(0o444)
PL102,
# os.mkdir("foo") should be replaced by foo_path.mkdir()
PL103,
# os.makedirs("foo/bar") should be replaced by bar_path.mkdir(parents=True)
PL104,
# os.rename("foo", "bar") should be replaced by foo_path.rename(Path("bar"))
PL105,
# os.replace("foo", "bar") should be replaced by foo_path.replace(Path("bar"))
PL106,
# os.rmdir("foo") should be replaced by foo_path.rmdir()
PL107,
# os.remove("foo") should be replaced by foo_path.unlink()
PL108,
# os.unlink("foo") should be replaced by foo_path.unlink()
PL109,
# os.getcwd() should be replaced by Path.cwd()
PL110,
# os.path.exists("foo") should be replaced by foo_path.exists()
PL111,
# os.path.expanduser("~/foo") should be replaced by foo_path.expanduser()
PL112,
# os.path.isdir("foo") should be replaced by foo_path.is_dir()
PL113,
# os.path.isfile("foo") should be replaced by foo_path.is_file()
PL114,
# os.path.islink("foo") should be replaced by foo_path.is_symlink()
PL115,
# os.readlink("foo") should be replaced by foo_path.readlink()
PL116,
# os.stat("foo") should be replaced by foo_path.stat() or foo_path.owner() or foo_path.group()
PL117,
# os.path.isabs should be replaced by foo_path.is_absolute()
PL118,
# os.path.join("foo", "bar") should be replaced by foo_path / "bar"
PL119,
# os.path.basename("foo/bar") should be replaced by bar_path.name
PL120,
# os.path.dirname("foo/bar") should be replaced by bar_path.parent
PL121,
# os.path.samefile("foo", "bar") should be replaced by foo_path.samefile(bar_path)
PL122,
# os.path.splitext("foo.bar") should be replaced by foo_path.suffix
PL123,
# open("foo") should be replaced by Path("foo").open()
PL124,
# py.path.local is in maintenance mode, use pathlib instead
# -------- https://github.com/c0ntribut0r/flake8-grug --------
GRG001,
# copy-paste
GRG002,
# early quit
GRG003,
# eval
GRG004,
# try too much
GRG005,
# requests no status check
Маленькие открытия, пока я всё это собирал:
Нашёл репо от команды BestDoctor. Они написали несколько плагинов для flake8, плюс вообще у них приятно - рекомендую python styleguide и погулять по репе. BestDoctor, оплату переводите на карту, как и договаривались.
flake8-simplify тоже вполне хорош, я джва года искал набор таких правил
Всякие code quality tools живут в PyCQA, могут пригодиться
Аннигилятор уже изобрели в wemake, но мне не все правила подходили, и у него фатальный недостаток
Когда после всех настроек я запустил аннигилятор, я понял: я создал такого же брюзгу, как я. Ура!
Только посмотрите:
И когда мой мега-линтер начал подсвечивать мой собственный код и бесить меня, я понял: он готов к работе.
Время запускать аннигилятор
Всё началось со статей Хабра, поэтому я решил соскрапать их все и скормить flake8. Я написал простую обёртку над Хабром и оставил на ночь. А потом запустил аннигилятор на всех постах.
flake8 --config .flake8 --format json samples/posts/ > results/posts-results.jsonВот цифры:
115,661 статья в потоке "разработка"
5,500 постов в Хабе "python"
1,723 поста про питон без кода или код меньше 5 строчек
¯\_(ツ)_/¯
Кодом я считал секции с тегом <code>. Иногда там не код, а командная строка или дурацкое форматирование, так что где flake8 смог переварить - хорошо, где не смог - не обессудьте.
Total posts: 2,791
Overall issues: 63,614
Unique issues (one per post): 12,166И вот, наконец, долгожданная статистика.
% of posts- процент постов, в которых ошибка встречалась хотя бы один разcnt- то же, но в абсолютном значении (количество)code- код ошибки в Flake8name- пример текста ошибки, может отличаться от случая к случаю
Ну что, поехали?
Мы прогуляемся по топчику из списка, и ещё я добавлю немного того, что не попало в список, потому что я не запрограммировал или запрограммировать невозможно.
Чтобы не сойти с ума, разобьём всё по секциям, упомянутым выше. Устранив ошибки секции, код можно будет
легко читать
легко изменять
легко отлаживать
легко тестировать
Легко читать
Легко читать > Самодокументация
Лень - двигатель прогресса. Ленивые программисты не любят писать документацию (да и кто вообще любит?), поэтому они стараются писать код, который сам себя документирует и его можно читать как книгу. Бонусом идёт то, что документация всегда в актуальном состоянии.
Легко читать > Самодокументация > Типы
Это питонушка. Тут в переменной может быть всё что угодно, хотите int, хотите гетерогенный список объектов вперемежку со словарями и числами с плавающей точкой. Считаю ли я это преимуществом питона? Нет. В большинстве случаев такая гибкость не нужна, и на этапе написания кода известно, где какие типы, так почему бы это не использовать ? (извините, не удержался от пропаганды)
Но мы имеем то, что имеем:
def fn(items):
# ...
Проблема: а что можно подавать в items, и что возвращается из fn?
Решение 1: смотреть код.
Топорное решение - посмотреть код и понять. И никто не гарантирует успех - может, вы поймёте, что items - это коллекция каких-то объектов, но вот чтобы понять, каких - нужно будет попрыгать по вызовам функции fn и посмотреть, что туда передаётся. Ну такое.
Решение 2: смотреть docstring.
Пещерные люди жили в неведении, но потом открыли для себя докстринги, где можно писать, что делает функция, что получает и что возвращает.
def fn(items):
"""
Sort items using mega-algo.
Args:
items: list - list of items
Returns:
list of sorted elements
"""
# ...
Это почти нормально, если бы не:
разные стили: где-то google-style, где-то sphinx-style
много писать (но помогают плагины)
нужно всё время следить за актуальностью: убрали аргумент, добавили аргумент, поменяли название - будьте добры обновить докстринг (я всё время забываю)
Решение 3: смотреть type hints
Потом пещерные люди посмотрели на статически типизированные языки и внезапно увидели, что это удобно, и захотели так же. Ну хотя бы чтоб было похоже. Поэтому вот:
from typing import List, Union, Optional
def fn(items: List[Optional[Union[int, float]]]) -> list[Union[int, float]]:
# ...
Смешно, что, хотя для items объявлен тип, по факту вы можете передать туда что угодно. Поэтому всё равно нужно следить, что куда передаётся, но, по крайней мере, это проще, и всякие линтеры могут помогать.
Решение 4: использовать python >= 3.10 и не импортировать лишнего
Наконец, пещерные люди посмотрели на List[Optional[Union[int, float]]], прифигели немного (это ведь только один аргумент!) и решили всё упростить:
def fn(items: list[int | float | None]) -> list[int, float]: # а ещё лучше TypeVar ;)
# ...
Согласитесь, эволюция налицо. Но проблемы всё равно есть:
type hints могут быть слишком большими. Постоянная тема:
dict[str, list[dict]]- это что вообще там? Если это тип для аргумента, то название аргумента подскажет, что имел в виду автор. А если это тип возвращаемого значения, то...return type hint никак не помогает!
Вот посмотрите сами:
def get_orientation(obj: Ufo) -> tuple[float, float, float]: # WAT? это что такое?
# ...
return pitch, roll, yaw # plot twist: я прочитал функцию до конца и узнал, что было в начале
Решение тут пока - в использовании namedtuple или иных заранее определённых типов для возвращаемого значения, но как по мне - слишком много boilerplate кода:
from collections import namedtuple
Orientation = namedtuple('FunctionResult', ('pitch', 'roll', 'yaw'))
def get_orientation(obj: Ufo) -> Orientation:
# ...
return Orientation(
pitch=...,
roll=...,
yaw=...,
)
Выглядит ужасно. Да здравствует питон!
Решение 5?
И пока пещерные люди выходят на новый виток эволюции, большинство разработчиков вообще живут мимо эволюции и не используют type annotations - в 80% постов они отсутствуют.
0 80% 2252 ANN Missing type annotation for function argument 'self'
1 77% 2154 TAE001 too few type annotations (0%)
Почему? Я не знаю. Питон 3.5 (а с ним и аннотации типов) вышел 7 лет назад!
Поэтому я напишу это здесь ещё раз: пожалуйста, используйте type hints. Это не сложно (кроме Generator[yield type, send type, return type], они реально бесят).
Легко читать > Самодокументация > Название функции
Ладно, вот вам новая проблема: что делает функция?
def fn(items: list[int | float | None]) -> list[int, float]:
Правильный ответ: а хрен её знает!
Решение 1: читать код
def fn(items: list[int]) -> list[int]:
return sorted(items, key=math.sin)
Сложность: O(n). Нет, пожалуйста.
Решение 2: читать docstring
def fn(items: list[int]) -> list[int]:
"""Sorts items by their sin() values"""
# ...
Это удобнее, но, как я уже говорил, эти докстринги должны быть, причём в актуальном состоянии.
Решение 3: нормально называть
def sorted_by_sin(items: list[int]) -> list[int]:
# ...
Тут сразу понятно: итемсы сортируются по грехам их. Стоп, что?
Ну короче вы поняли. Правильное название функции и короткий, читаемый код заменяют docstring. Признаться, раньше я был свидетелем докстринговым и везде старался их писать, но теперь я их пишу только во всяких публичных функциях публичных либ, чтобы документация красиво собиралась.
Вопрос в кассу: как проверить, что название хорошее? Ответ: люди в чёрном.
Берёте Уилла Смита, он стирает вам память, после чего показывает аннотацию функции. Если вы примерно угадываете, что функция делает - название хорошее.
Легко читать > Самодокументация > Названия переменных
Та же тема с названиями переменных. Дополнительно хочу отметить: никто вас не наругает, если вы используете несколько слов в названии переменной, и она вдруг от этого станет понятной.
Если хорошее название - дело субъективное, то вот переопределение встроенных символов (например, list, set, next) - очевидное зло. Чтобы вы не теряли бдительность, в 5% постов авторы переопределяют встроенные символы при помощи переменных, а в 3% - при помощи аргументов.
5% 150 A001 variable "xxx" is shadowing a python builtin
3% 100 A002 argument "xxx" is shadowing a python builtin
2% 72 CAC002 function is too complex. Bad variable names penalty is too highЯ написал скрипт, который по коду ошибки выдаёт примеры прям из статей с Хабра, так что вот живой пример: какой-то дурачок переопределил repr.
def repr(group_start, group_end) -> str: # <--------------------------------!!!
# это просто правильно печатает группу
if last_group_start == last_group_end:
return str(last_group_end)
Что-то знакомое... Блин, да это ж я! Так, забыли.
Как понять, что название хорошее?
Легко читать > Самодокументация > Комментарии
Тут всё просто: код рассказывает компьютеру, что делать. Когда человек читает код, он понимает, что компьютер будет делать. Поэтому в большинстве случаев комментировать это не надо. Такие комментарии только занимают место и не приносят никакой пользы.
# делаем проверку по индексу 26
data.loc[26]Но вот если после Уилла Смита вы не понимаете, что делает этот код (или зачем) - комментарий будет совсем не лишним:
Легко читать > Low RAM
Я уже говорил, что у меня в голове место ограничено, поэтому я стараюсь как можно сильнее снизить когнитивную нагрузку - иначе говоря, минимизировать количество информации, которую мне нужно держать в голове. Для этого я стараюсь уменьшать, разбивать и упрощать функции.
Легко читать > Low RAM > Невысокая функция
Тут всё просто: чем короче функция, тем проще её понять.
Разбитие функции
Функция слишком длинная? Рецепт прост: разбейте её на несколько! Увы, в 16% постов авторы пишут мега-функции:
16% 447 CCR001 Cognitive complexity is too high
12% 359 CAC001 function is too complexНу вот, например:
def run(self):
pixels = pygame.surfarray.pixels3d(self.display)
index = 0
running = True
while running:
self.model.stimulate()
pixels[:, :, :] = (255 * (self.model.activity > 0))[:, :, None]
pygame.display.flip()
for event in pygame.event.get():
if event.type == pygame.QUIT:
running = False
elif event.type == pygame.KEYDOWN:
if event.key == pygame.K_ESCAPE:
running = False
elif event.key == pygame.K_s:
imsave("{0:04d}.png".format(index), pixels[:, :, 0])
index = index + 1
elif event.type == pygame.MOUSEBUTTONDOWN:
position = pygame.mouse.get_pos()
self.model.activity[position] = 1
Тут игровой цикл, где смешались вызов симуляции, отрисовка пикселей, обработка клавиатуры, скриншоты... Как упростить? Разбейте на функции:
def run(self):
while self.is_running:
self.model.simulate()
self.draw()
self.handle_input()
Группировка присваиваний в one-liner
Магическое превращение четырёх строчек в одну! Следите за руками:
if a > b:
max_ = a
else:
max_ = b
# ...вжух!
max_ = a if a > b else b
Я люблю такие присваивания, потому что они занимают меньше места. Но ещё важная фишка в том, что у меня одно присваивание в коде вместо двух, значит, меньше шансов на ошибку.
Легко читать > Low RAM > Неширокая функция
Когда у вас слишком много отступов в коде - это знак, что что-то можно упростить. Например:
Циклы
Если у вас в цикле много всего, то можно это всё вынести в отдельную функцию. Вы не только упрощаете код, но и "сдвигаете" всё из цикла на один уровень вложенности меньше:
def run():
for event in pygame.event.get():
if event.type == pygame.QUIT: # тут уровень отступов == 2
running = False # тут уровень отступов == 3
# ...
# ...вжух!
def process_event(event: Event):
if event.type == pygame.QUIT: # тут уровень отступов == 1
running = False # тут уровень отступов == 2
# ...
def run():
for event in pygame.event.get():
process_event(event)
Мини-бонус: если в цикле вызывается одна функция, то этот цикл очень просто распараллелить (при помощи ThreadPoolExecutor, например).
Длинные выражения
Ну любит народ писать зубодробительные выражения (11%):
11% 325 ECE001 Expression is too complex (13.5 > 7)Вот, например:
# constraints on communist's or zhulik's votes
def findVariants(s, aim, cnt, suffix):
s.add((r9916['com'] - aim['com'] == \ # <--------------------------------!!!
Sum([If(Bool('v_%d_%s' % (i, suffix)), rewritten_pecs[i]['increase']['com'], 0) for i in range(19)]) + \
Sum([If(Bool('a_%d_%s' % (k, suffix)), 0, approved_pecs[k]['first']['com']) for k in range(53)])))
Я не знаю, что можно сделать с выражением выше, но если у вас какая-то строчка эпической длины, то хотя бы попробуйте разбить её на несколько - так легче читается. В этом очень помогают скобки, ведь то, что нельзя делать в обычном мире, в мире скобок дозволено.
Вот тут скобки позволяют писать + что-то с новой строчки, да ещё и комментарии можно добавлять:
missing_set = ( # <--------------------------------!!!
[(all_letters, '-' * len(all_letters))] * 3 # тут считаем все буквы пропущенными
+ [(all_letters, all_letters)] * 10 # тут считаем все буквы НЕ пропущенными
+ [('aeiouy', '------')] * 30 # тут считаем пропущенными только гласные
)
Или вот простыня текста разрулена скобками:
fragment = (
'Абсолютная идея есть для себя, потому что в ней нет ни перехода, '
'ни предпосылок и вообще никакой определенности, которые не были бы '
'текучи и прозрачны; она есть чистая форма понятия, которая созерцает '
'свое содержание как самое себя. Она есть свое собственное содержание, '
'поскольку она есть идеальное различение самой себя от себя, и одно из '
'этих различий есть тождество с собой, которое, однако, содержит в себе '
'тотальность форм как систему содержательных определений. '
'Это содержание есть система логического.'
f'({author})'
)
Nested conditions
Тут всё просто: зачем два if, если можно один? Вроде банальщина, а тем не менее - 2%:
2% 65 SIM102 Use a single if-statement instead of nested if-statementsif min_val is not None and max_val is not None:
if max_val < min_val:
raise ValueError("max_val is greater than min_val") # тут уровень отступов == 2
# ...вжух!
if min_val is not None and max_val is not None and max_val < min_val:
raise ValueError("max_val is greater than min_val") # тут уровень отступов == 1
Pathlib
pathlib (о котором я замолвлю словечко позже) позволяет читать и писать в одну строчку:
with open('test.txt') as file:
content = open.read() # тут уровень отступов == 1
# ...вжух!
content = Path('test.txt').read_text() # тут уровень отступов == 0Redundant else
Один из моих любимых способов уменьшения отступов: если в теле после if у вас return или raise, то else не нужен. Проще на примере:
if user_profile:
# ...
return HttpResponseRedirect(reverse("main:dashboard")) # или raise
else:
# ...
messages.success(request, _("Успешная регистрация.")) # тут уровень отступов == 1
return HttpResponseRedirect(reverse("main:dashboard"))
# ...вжух!
if user_profile:
# ...
return HttpResponseRedirect(reverse("main:dashboard"))
# ...
messages.success(request, _("Успешная регистрация.")) # тут уровень отступов == 0
return HttpResponseRedirect(reverse("main:dashboard"))
Легко читать > Low RAM > Небольшой контекст
Каждый if заставляет вас запоминать, что было ветвление на 2 или более случая. Каждая переменная заставляет вас помнить, что она есть и в ней что-то лежит. Каждый try заставляет ожидать исключения в каждой строчке до самого except. Вроде пустяки, но чем их больше, тем сложнее за всем уследить. Всё, что нужно держать в голове, я называю "контекстом", и в этой секции мы будем его уменьшать.
Early quit
Я уже рассказывал про него, но это такой классный паттерн, что я даже добавил его в плагин для flake8 (правда, там много false positives). Правило такое: старайтесь выйти из функции / цикла как можно быстрее. На любой развилке if-else первым проверяйте то условие, которое позволит выйти из тела функции / цикла.
Пример:
if rate is not None:
# в этом месте мы должны помнить, что есть ещё случай `if rate is None`, и его нужно потом рассмотреть
r1 = (df[minus] / df[column] - 1).abs()
r2 = (df[plus] / df[column] - 1).abs()
return df.loc[(df['new_cases'].ge(100)) & ((r1.notnull() & (r1 <= rate)) | (r2.notnull() & (r2 <= rate)))]
else: # и вот мы наконец его рассматриваем
raise Exception
# ...вжух!
if rate is None: # сначала рассматриваем случай, который нас выкинет из текущего контекста
raise Exception
# если не выкинул - то хотя бы не нужно про него больше помнить
r1 = (df[minus] / df[column] - 1).abs()
r2 = (df[plus] / df[column] - 1).abs()
return df.loc[(df['new_cases'].ge(100)) & ((r1.notnull() & (r1 <= rate)) | (r2.notnull() & (r2 <= rate)))]
Или вот:
if response.status == httplib.OK:
parser = DNSParser()
parser.feed(response.read())
return parser.saved_data
else:
return 'Error ' + str(response.status)
# ...вжух!
if response.status != httplib.OK:
return 'Error ' + str(response.status)
parser = DNSParser()
parser.feed(response.read())
return parser.saved_data
Обобщая, после этой оптимизации код должен выглядеть так:
если что-то не так:
return
если что-то ещё не так:
return
если ошибка:
raise
# тут мы уверены, что всё в порядке
код, делающий что-то полезное
Small try blocks
В 4% постов авторы пытаются скормить в try как можно больше всего. Наверно, это какие-то корни рыболовов дают о себе знать - натянул сеть пошире и ловишь себе всё, что есть, иногда даже не гнушаясь ловить просто Exception.
4% 106 GRG004 Too big "try" code blockВ идеале вы должны знать, что конкретно и где вы ловите - тогда при срабатывании блока except вы точно будете знать, из какой строчки что прилетело. Поэтому в идеале между try и except должно быть 1-3 строчки, а если больше - задумайтесь, возможно, вы не понимаете, что у вас происходит в коде.
Кстати, try-except вводят дополнительный отступ, а чем больше отступов... ну вы помните.
def new_from_file(self, filename, selected=True):
try:
file_path = normalize_path(filename, True)
obj = DesktopParser(file_path)
sname = obj.get('Name',locale=LOCALE)
desc = obj.get('Comment',locale=LOCALE)
icon = obj.get('Icon')
pix = IconManager().get_icon(ThemedIcon('image-missing'),32)
if icon:
if icon.rfind('.') != -1:
pix = IconManager().get_icon(FileIcon(File(icon)),32)
else:
pix = IconManager().get_icon(ThemedIcon(icon),32)
data = (pix, '%s' % sname, obj, sname.upper(), file_path)
return self.launcher_view.add_row(data,selected)
except:
return None
# ...вжух!
def new_from_file(self, filename, selected=True):
file_path = normalize_path(filename, True)
try:
obj = DesktopParser(file_path)
except DesktopParserError:
return
sname = obj.get('Name',locale=LOCALE)
desc = obj.get('Comment',locale=LOCALE)
icon = obj.get('Icon')
try:
pix = IconManager().get_icon(ThemedIcon('image-missing'),32)
except IconError:
return
if icon:
if icon.rfind('.') != -1:
pix = IconManager().get_icon(FileIcon(File(icon)),32)
else:
pix = IconManager().get_icon(ThemedIcon(icon),32)
data = (pix, '%s' % sname, obj, sname.upper(), file_path)
return self.launcher_view.add_row(data,selected)
Если у вас прям какая-то ненадёжная функция и вы хотите весь её код завернуть в try-except, то хотя бы юзайте декоратор.
Context manager
7% 208 SIM115 Use context handler for opening filesВнезапно, в 7% постов авторы обходятся без контекстных менеджеров для открытия файлов. Они, наверно, супермены и всегда помнят, что надо закрывать файлы?
@profile
def create_file():
x = [(random.random(),
random_string(),
random.randint(0, 2 ** 64))
for _ in xrange(1000000)]
pickle.dump(x, open('machin.pkl', 'w'))
Я вот могу и забыть про file.close(). Эти ваши интернеты говорят, что если самому не вызвать close(), то cpython закрывает файлы, когда их refcount = 0 и сборщик мусора добирается до них, а вот pypy - только при завершении процесса. Так что лучше использовать контекстные менеджеры, и пусть оно там само помнит, когда что закрыть и отключить:
with open('machin.pkl', 'w') as out_file:
pickle.dump(x, out_file)
Здесь мы рассмотрели контекстный менеджер для open(), но есть и контекстные менеджеры для разного - соединения с базой данных, к примеру - полезные штуки!
Yagni
В коде не должно быть ненужного. Если у вас код под VCS, то смело выбрасывайте мёртвый код - потому что потом можно будет всегда вернуть (но, скорей всего, до этого не дойдёт). Ну и просто по мелочи:
Заменяйте ненужные переменные на underscore
10% 285 B007 Loop control variable 'i' not used within the loop body. If this is intended, start the name with an underscore.for i in range(32):
print(quantum_randbit(), end='')
# ...вжух!
for _ in range(32):
print(quantum_randbit(), end='')
Для путешественников во времени: у нас в 2022 object уже необязателен.
5% 160 SIM120 Use 'class Bot:' instead of 'class Bot(object):'class DataItem(object):
__slots__ = ['name', 'age', 'address']
# ...вжух!
class DataItem:
__slots__ = ['name', 'age', 'address']
Не надо создавать контейнер, а потом наполнять его. Наполняйте сразу!
1% 44 PIE799 prefer-col-init: Consider passing values in when creating the collection.Contour=[]
Contour.append( [pcbnew.Millimeter2iu(plane_size[0]), pcbnew.Millimeter2iu(plane_size[1]) ])
Contour.append( [pcbnew.Millimeter2iu(plane_size[0]), -pcbnew.Millimeter2iu(plane_size[1]) ])
Contour.append( [-pcbnew.Millimeter2iu(plane_size[0]), -pcbnew.Millimeter2iu(plane_size[1]) ])
Contour.append( [-pcbnew.Millimeter2iu(plane_size[0]), pcbnew.Millimeter2iu(plane_size[1]) ])
# ...вжух!
Countour = [
[pcbnew.Millimeter2iu(plane_size[0]), pcbnew.Millimeter2iu(plane_size[1]) ],
[pcbnew.Millimeter2iu(plane_size[0]), -pcbnew.Millimeter2iu(plane_size[1]) ],
[-pcbnew.Millimeter2iu(plane_size[0]), -pcbnew.Millimeter2iu(plane_size[1]) ],
[-pcbnew.Millimeter2iu(plane_size[0]), pcbnew.Millimeter2iu(plane_size[1]) ],
]
Легко читать > Low RAM > Best practices
А ещё код легко читать, если его пишут так, как общепринято и актуально по нынешней спецификации.
Range
Внезапно для 4%, range уже начинается с нуля!
4% 119 PIE808 prefer-simple-range: range starts at 0 by default.for i in range(0, n):
dydt[i] = y[i + n]
# ...вжух!
for i in range(n):
dydt[i] = y[i + n]
Enumerate
Если в цикле вы используете счётчик и увеличиваете его на 1 в конце каждой итерации, то, скорее всего, доктор вам пропишет enumerate().
2% 70 SIM113 Use enumerate for 'val'counter = 0
for vk in vk_apis:
change_photo(vk)
print(counter, 'done')
counter += 1
# ...вжух!
for i, vk in enumerate(vk_apis):
change_photo(vk)
print(i, 'done')
Suppress
1% 55 SIM105 Use 'contextlib.suppress(Exception)'Сам недавно узнал! Позволяет укоротить try-except-pass:
try:
flatPriceDelta = float(flatPriceInfo) - float(oldFlatPriceInfo)
except ValueError:
pass
# ...вжух!
with suppress(ValueError):
flatPriceDelta = float(flatPriceInfo) - float(oldFlatPriceInfo)
Pathlib
pathlib - это как type hints: полезная штука, которую почему-то часто игнорируют. Если вы используете os.path, то время сесть в делореан, переместиться в будущее и начать пользоваться pathlib.
1% 41 PL107 os.remove("foo") should be replaced by foo_path.unlink()
1% 38 PL103 os.makedirs("foo/bar") should be replaced by bar_path.mkdir(parents=True)
14% 395 PL123 open("foo") should be replaced by Path("foo").open()
3% 96 PL118 os.path.join("foo", "bar") should be replaced by foo_path / "bar"
2% 67 PL110 os.path.exists("foo") should be replaced by foo_path.exists()os.remove("../m3u8_downloader/segments/temp.ts")
# ...вжух!
Path("../m3u8_downloader/segments/temp.ts").unlink()
if not os.path.exists(tools_path):
os.makedirs(tools_path)
# ...вжух!
tools_path.mkdir(parents=True, exist_ok=True)
with open('posts.txt') as file:
# ...вжух!
with Path('posts.txt').open() as file:
def SaveDriverFile(self):
winPath = os.environ['WINDIR']
sys32Path = os.path.join(winPath, "System32")
targetPath = os.path.join(sys32Path, "drivers\\" + self.name + ".sys")
file_data = open(self.file_path, "rb").read()
open(targetPath, "wb").write(file_data)
# ...вжух!
def SaveDriverFile(self):
winPath = Path(os.environ['WINDIR'])
sys32Path = winPath / "System32"
targetPath = sys32Path / "drivers" / self.name + ".sys"
file_data = self.file_path.read_bytes()
targetPath.write_bytes(file_data)
if not os.path.exists(HOME_DIR+'/'+'ShootAndView'):
# ...вжух!
if not (HOME_DIR / 'ShootAndView').exists():
Кстати, pathlib можно использовать в argparse:
from argparse import ArgumentParser
parser = ArgumentParser()
parser.add_argument('path', type=Path)
args = parser.parse_args()
subpath = args.path / 'subpath'
Boolean expressions
1% 31 PIE801 prefer-simple-return: Return boolean expressions directly instead of returning `True` and `False`.if q.measure()[:~0] == '0' * n:
return True
else:
return False
# ...вжух!
return q.measure()[:~0] == '0' * n
Filter / map
1% 37 C417 Unnecessary use of map - use a generator expression instead.В большинстве случаев filter и map легко заменяются на генераторы или list comprehension, а бонусом приходит лучшая читаемость.
key_dicts = map(lambda key: key.as_key(), keys)
return dict(keys=list(key_dicts), nextApplicationKeyId=next_application_key_id)
# ...вжух!
key_dicts = [key.as_key() for key in keys]
return dict(keys=key_dicts, nextApplicationKeyId=next_application_key_id)
Настоящие злодеи используют filter(map(...)), вот вам мой шедевр:
params = {
'--write-buffer-size': 123,
'--skip-hash-verification': None,
'--max-download-streams-per-file': 8,
}
args = list(map(str, filter(None, chain.from_iterable(params.items()))))
Не делайте так.
Легко читать > Low RAM > Well-known libs
Чтобы не писать зубодробительный код, можно использовать библиотеки, которые этот код уже написали. Тут я опишу лишь те, о которых не могу промолчать.
Requests
Да-да, мы все их используем, но многие не знают двух фишек:
requests.get(...)создаёт новую сессию для каждого вызова, что накладно, если вы делаете запросы к одному и тому же сайту. Правильный подход - использовать сессии, но описано это почему-то в "advanced usage"¯\_(ツ)_/¯По умолчанию нет никакого таймаута на запрос, поэтому если сайт долго отвечает, то ваш код тоже будет тормозить.
Tenacity
После того, как я увидел tenacity, я не хочу пользоваться никакими другими retry-библиотеками. Только посмотрите, как он легко читается:
@retry(
reraise=True,
wait=wait_incrementing(start=1, increment=2),
stop=stop_after_attempt(20),
)
def request(self, method: str, path: str, **kwargs) -> requests.Response:
if path.startswith('/'):
path = self.BASE_URL + path
kwargs.setdefault('timeout', self.TIMEOUT)
response = self.session.request(method, path, **kwargs)
if response.status_code == requests.codes.too_many_requests:
raise TryAgain()
return response
Пожалуйста, не изобретайте велосипеды, если вам нужно запилить retry логику. Используйте tenacity.
More-itertools
Если вы не слышали про itertools из стандартной библиотеки, то самое время почитать. itertools.chain меня постоянно выручает.
Но если itertools вам уже не хватает, то вэлкам: more-itertools. Тут есть chunked, spy, first, one, only, unique_everseen и прочие прелести. Осторожно, с этой дряни невозможно слезть.
Легко изменять
Может, вы сами, а может, кто-то из интернета однажды захочет что-то изменить в вашем коде. И этот кто-то мысленно скажет вам спасибо, если вы заранее позаботитесь об этом.
Легко изменять > Классы
Внезапно, классы полезны не только для инкапсуляции состояния объекта, но и для наследования! Вот, например, просто функция:
SOME_CONSTANT = 5
def do_something(value: int) -> int:
return value * SOME_CONSTANT
Как сделать, чтоб функция работала с другой константой? Передавать константу явно в функцию и в каждом месте, где вызывается функция, передавать новую константу. Или определить new_do_something = partial(do_something, constant=6) и везде использовать новую функцию. Но...
Вот то же, но через класс. Теперь кто угодно может переопределить и константу, и функцию, и всё вместе, и надстроить что-то поверх.
class Something:
SOME_CONSTANT = 5
@classmethod
def do_something(cls, value: int) -> int:
return value * cls.SOME_CONSTANT
class Other(Something):
SOME_CONSTANT = 6
@classmethod
def do_something(cls, value: int) -> int:
result = super().do_something(value)
return result - 3
Не то чтобы я топил за использование классов везде - иногда это излишне, и старые добрые функции будут решать задачу без проблем. Но если планируется какое-то переиспользование кода - я бы смотрел в сторону классов.
Легко изменять > Функции
Я прям сразу вспоминаю внутренности Django. Не дай бог мне вдруг не понравится что-то в стандартной инициализации модели - вы только посмотрите, какое там полотно!
base.py (осторожно, опасно для глаз)
class ModelBase(type):
"""Metaclass for all models."""
def __new__(cls, name, bases, attrs, **kwargs):
super_new = super().__new__
# Also ensure initialization is only performed for subclasses of Model
# (excluding Model class itself).
parents = [b for b in bases if isinstance(b, ModelBase)]
if not parents:
return super_new(cls, name, bases, attrs)
# Create the class.
module = attrs.pop("__module__")
new_attrs = {"__module__": module}
classcell = attrs.pop("__classcell__", None)
if classcell is not None:
new_attrs["__classcell__"] = classcell
attr_meta = attrs.pop("Meta", None)
# Pass all attrs without a (Django-specific) contribute_to_class()
# method to type.__new__() so that they're properly initialized
# (i.e. __set_name__()).
contributable_attrs = {}
for obj_name, obj in attrs.items():
if _has_contribute_to_class(obj):
contributable_attrs[obj_name] = obj
else:
new_attrs[obj_name] = obj
new_class = super_new(cls, name, bases, new_attrs, **kwargs)
abstract = getattr(attr_meta, "abstract", False)
meta = attr_meta or getattr(new_class, "Meta", None)
base_meta = getattr(new_class, "_meta", None)
app_label = None
# Look for an application configuration to attach the model to.
app_config = apps.get_containing_app_config(module)
if getattr(meta, "app_label", None) is None:
if app_config is None:
if not abstract:
raise RuntimeError(
"Model class %s.%s doesn't declare an explicit "
"app_label and isn't in an application in "
"INSTALLED_APPS." % (module, name)
)
else:
app_label = app_config.label
new_class.add_to_class("_meta", Options(meta, app_label))
if not abstract:
new_class.add_to_class(
"DoesNotExist",
subclass_exception(
"DoesNotExist",
tuple(
x.DoesNotExist
for x in parents
if hasattr(x, "_meta") and not x._meta.abstract
)
or (ObjectDoesNotExist,),
module,
attached_to=new_class,
),
)
new_class.add_to_class(
"MultipleObjectsReturned",
subclass_exception(
"MultipleObjectsReturned",
tuple(
x.MultipleObjectsReturned
for x in parents
if hasattr(x, "_meta") and not x._meta.abstract
)
or (MultipleObjectsReturned,),
module,
attached_to=new_class,
),
)
if base_meta and not base_meta.abstract:
# Non-abstract child classes inherit some attributes from their
# non-abstract parent (unless an ABC comes before it in the
# method resolution order).
if not hasattr(meta, "ordering"):
new_class._meta.ordering = base_meta.ordering
if not hasattr(meta, "get_latest_by"):
new_class._meta.get_latest_by = base_meta.get_latest_by
is_proxy = new_class._meta.proxy
# If the model is a proxy, ensure that the base class
# hasn't been swapped out.
if is_proxy and base_meta and base_meta.swapped:
raise TypeError(
"%s cannot proxy the swapped model '%s'." % (name, base_meta.swapped)
)
# Add remaining attributes (those with a contribute_to_class() method)
# to the class.
for obj_name, obj in contributable_attrs.items():
new_class.add_to_class(obj_name, obj)
# All the fields of any type declared on this model
new_fields = chain(
new_class._meta.local_fields,
new_class._meta.local_many_to_many,
new_class._meta.private_fields,
)
field_names = {f.name for f in new_fields}
# Basic setup for proxy models.
if is_proxy:
base = None
for parent in [kls for kls in parents if hasattr(kls, "_meta")]:
if parent._meta.abstract:
if parent._meta.fields:
raise TypeError(
"Abstract base class containing model fields not "
"permitted for proxy model '%s'." % name
)
else:
continue
if base is None:
base = parent
elif parent._meta.concrete_model is not base._meta.concrete_model:
raise TypeError(
"Proxy model '%s' has more than one non-abstract model base "
"class." % name
)
if base is None:
raise TypeError(
"Proxy model '%s' has no non-abstract model base class." % name
)
new_class._meta.setup_proxy(base)
new_class._meta.concrete_model = base._meta.concrete_model
else:
new_class._meta.concrete_model = new_class
# Collect the parent links for multi-table inheritance.
parent_links = {}
for base in reversed([new_class] + parents):
# Conceptually equivalent to `if base is Model`.
if not hasattr(base, "_meta"):
continue
# Skip concrete parent classes.
if base != new_class and not base._meta.abstract:
continue
# Locate OneToOneField instances.
for field in base._meta.local_fields:
if isinstance(field, OneToOneField) and field.remote_field.parent_link:
related = resolve_relation(new_class, field.remote_field.model)
parent_links[make_model_tuple(related)] = field
# Track fields inherited from base models.
inherited_attributes = set()
# Do the appropriate setup for any model parents.
for base in new_class.mro():
if base not in parents or not hasattr(base, "_meta"):
# Things without _meta aren't functional models, so they're
# uninteresting parents.
inherited_attributes.update(base.__dict__)
continue
parent_fields = base._meta.local_fields + base._meta.local_many_to_many
if not base._meta.abstract:
# Check for clashes between locally declared fields and those
# on the base classes.
for field in parent_fields:
if field.name in field_names:
raise FieldError(
"Local field %r in class %r clashes with field of "
"the same name from base class %r."
% (
field.name,
name,
base.__name__,
)
)
else:
inherited_attributes.add(field.name)
# Concrete classes...
base = base._meta.concrete_model
base_key = make_model_tuple(base)
if base_key in parent_links:
field = parent_links[base_key]
elif not is_proxy:
attr_name = "%s_ptr" % base._meta.model_name
field = OneToOneField(
base,
on_delete=CASCADE,
name=attr_name,
auto_created=True,
parent_link=True,
)
if attr_name in field_names:
raise FieldError(
"Auto-generated field '%s' in class %r for "
"parent_link to base class %r clashes with "
"declared field of the same name."
% (
attr_name,
name,
base.__name__,
)
)
# Only add the ptr field if it's not already present;
# e.g. migrations will already have it specified
if not hasattr(new_class, attr_name):
new_class.add_to_class(attr_name, field)
else:
field = None
new_class._meta.parents[base] = field
else:
base_parents = base._meta.parents.copy()
# Add fields from abstract base class if it wasn't overridden.
for field in parent_fields:
if (
field.name not in field_names
and field.name not in new_class.__dict__
and field.name not in inherited_attributes
):
new_field = copy.deepcopy(field)
new_class.add_to_class(field.name, new_field)
# Replace parent links defined on this base by the new
# field. It will be appropriately resolved if required.
if field.one_to_one:
for parent, parent_link in base_parents.items():
if field == parent_link:
base_parents[parent] = new_field
# Pass any non-abstract parent classes onto child.
new_class._meta.parents.update(base_parents)
# Inherit private fields (like GenericForeignKey) from the parent
# class
for field in base._meta.private_fields:
if field.name in field_names:
if not base._meta.abstract:
raise FieldError(
"Local field %r in class %r clashes with field of "
"the same name from base class %r."
% (
field.name,
name,
base.__name__,
)
)
else:
field = copy.deepcopy(field)
if not base._meta.abstract:
field.mti_inherited = True
new_class.add_to_class(field.name, field)
# Copy indexes so that index names are unique when models extend an
# abstract model.
new_class._meta.indexes = [
copy.deepcopy(idx) for idx in new_class._meta.indexes
]
if abstract:
# Abstract base models can't be instantiated and don't appear in
# the list of models for an app. We do the final setup for them a
# little differently from normal models.
attr_meta.abstract = False
new_class.Meta = attr_meta
return new_class
new_class._prepare()
new_class._meta.apps.register_model(new_class._meta.app_label, new_class)
return new_class
Чтобы пропатчить одну строчку в этой функции, мне нужно полностью эту функцию переписать! А потом в следующем релизе разрабы джанго там что-то поменяют, и мой патч сломается.
Мораль? Чем меньше метод, тем легче его заменить.
Легко изменять > Переменные
Как я уже говорил выше, если вы хотите, чтобы можно было изменять переменные - то либо делайте их class variables, либо передавайте явно как параметр в функцию. Никаких magic numbers в теле функции!
# бро
class Some:
CONST = 5
# бро
def do(const: int = 5):
# ...
# не бро
def do():
for i in range(5):
# ...
Легко изменять > Dependency injection
Справедливости ради следует упомянуть dependency injection, как вариант, облегчающий модификацию и тестирование кода, но, кажется, за меня про это уже написали тут.
Легко дебажить
Легко дебажить - это значит иметь место для брейкпоинта, и чтоб можно было понять, что вообще происходит.
Легко дебажить > except или except Exception
Я уже упоминал про моряков, которые ловят всё. И это плохо, потому что, отлавливая всё подряд, вы пытаетесь пофиксить своё непонимание - вы не понимаете, какие исключения могут прилететь. Разберитесь в коде, а не пытайтесь поставить заплатку!
6% 171 B001 Do not use bare `except:`, it also catches unexpected events like memory errors, interrupts, system exit, and so on.
9% 274 PIE786 Use precise exception handlers.for node in G.nodes():
try:
labels[node]=names['keysets']['generated'][nodeStdict[node]+'-name']['ru']
except: labels[node]='error'
# ...вжух!
for node in G.nodes():
try:
labels[node]=names['keysets']['generated'][nodeStdict[node]+'-name']['ru']
except KeyError:
labels[node] = 'error'
Легко дебажить > mega-statements
Чем длиннее выражение, тем сложнее его отлаживать.
Например, в этом случае нет места для брейкпоинта, потому что мега-объект создаётся несколько раз в цикле из itertools.product, и сразу с ним делается assert. Это просто ужасно!
assert replication_status_json in [
{
"replication-one":
[
{
"count": 1,
"destination_replication_status": None,
"hash_differs": None,
"metadata_differs": None,
"source_has_file_retention": None,
"source_has_hide_marker": None,
"source_has_large_metadata": None,
"source_has_legal_hold": None,
"source_has_sse_c_enabled": None,
"source_replication_status": None,
}
],
"replication-two":
[
{
"count": 1,
"destination_replication_status": None,
"hash_differs": None,
"metadata_differs": None,
"source_has_file_retention": False,
"source_has_hide_marker": False,
"source_has_large_metadata": False,
"source_has_legal_hold": True,
"source_has_sse_c_enabled": False,
"source_replication_status": first,
}, {
"count": 1,
"destination_replication_status": None,
"hash_differs": None,
"metadata_differs": None,
"source_has_file_retention": False,
"source_has_hide_marker": False,
"source_has_large_metadata": False,
"source_has_legal_hold": False,
"source_has_sse_c_enabled": False,
"source_replication_status": second,
}, {
"count": 1,
"destination_replication_status": None,
"hash_differs": None,
"metadata_differs": None,
"source_has_file_retention": False,
"source_has_hide_marker": False,
"source_has_large_metadata": False,
"source_has_legal_hold": False,
"source_has_sse_c_enabled": True,
"source_replication_status": None,
}, {
"count": 1,
"destination_replication_status": None,
"hash_differs": None,
"metadata_differs": None,
"source_has_file_retention": False,
"source_has_hide_marker": False,
"source_has_large_metadata": False,
"source_has_legal_hold": True,
"source_has_sse_c_enabled": True,
"source_replication_status": None,
}
]
} for first, second in itertools.product(['FAILED', 'PENDING'], ['FAILED', 'PENDING'])
]
Легко дебажить > думать вперёд
Иногда можно писать сразу так, чтобы потом не дебажить :)
Copy-paste
Хотите сделать опечатку? Юзайте copy-paste! За пруфами отсылаю вас к PVS Studio, а на Хабре... на Хабре копи-пасту можно найти очень часто.
44% 715 GRG001 Copy-paste of codeИ хотя в моём плагине много false positives, проблема действительно есть:
xn = fincoords[0][:,0]
yn = fincoords[0][:,1]
zn = fincoords[0][:,2]
l=[labels[idi] for idi in myIDs]
Чем больше строчек скопировано, тем легче будет ошибиться - забыть поменять что-то в одной из строчек. Решение - выделять общий функционал в цикл / отдельную функцию.
Response status
Вот вам забавный факт: в 4% постов разработчику вообще плевать, что там вернул сервер. Не стоит прогибаться под изменчивый сервер, пусть лучше он прогнётся под нас!
4% 111 GRG005 Not checking response status codedef uncapcha(url):
imager = requests.get(url)
r = requests.post('http://rucaptcha.com/in.php', data = {'method': 'base64', 'key': RUCAPTCHA_KEY, 'body': base64.b64encode(imager.content)})
if (r.text[:3] != 'OK|'):
print('captcha failed')
return -1
capid = r.text[3:]
sleep(5)
response.ok или response.raise_for_status() в помощь!
Проблема print() в том, что им очень просто пользоваться. Вот все им и пользуются, но через какое-то время возникает другая проблема: а как его заткнуть?
Поэтому я рекомендую сразу, прям с самого начала использовать модуль logging и разделять вывод на debug, info, warning и error. Тогда во время отладки программы вы будете выводить все сообщения, даже отладочные, а потом просто смените уровень на info или warning, и отладочные сообщения не будут засорять вам вывод.
44% 1246 T201 print found.async def fun2(x):
print(x**0.5)
await asyncio.sleep(3)
print('fun2 завершена')
# ...вжух!
import logging
log = logging.getLogger(__name__)
async def fun2(x):
print(x**0.5)
await asyncio.sleep(3)
log.debug('fun2 завершена')
if __name__ == '__main__':
logging.basicConfig(level=logging.DEBUG)
Легко тестировать
Легко тестировать
Как сделать тестирование лёгким и приятным? А вот как: пишите функции, которые
ни от чего не зависят, кроме входных параметров
делают только одну вещь
без side effects, а если и с ними, то это единственное их назначение
Я собрал джекпот для примера:
items = [1, 2]
def append_double(el: int) -> int:
# функция имеет side effect: изменяет items
double = el * 2
items.append(double) # функция зависит от глобальной переменной items
return double # помимо side effect, функция ещё что-то возвращаетДля всех случаев у меня нет авто-детекторов, но вот статистика по использованию ключевого слова global:
3% 98 W002 Global variable next usedНе делайте так!
def format_time(x, pos=None):
global duration, nframes, k
progress = int(x / float(nframes) * duration * k)
# ...Время статистики
Давайте применим наше знание, чтобы что-нибудь понять.
Статистика по постам
Рейтинг от общего количества ошибок на пост:
Похоже, чем более грязный код вы пишете, тем меньше у вас шансов получить высокий рейтинг. Но это неточно.
Статистика по популярным питон-библиотекам
Ставил последние версии на 1.09.2022.
django
У меня с django довольно нежные отношения, поэтому мне было интересно: так ли там всё плохо, как мне кажется?
Хех, похоже, что да!
Во-первых, джанго кладёт огроменный болт на type annotations. У них так принято
python show-me-sample.py --results results/libs-results-django.json --order random --context 8 ANN
-------------------- samples/libs/django/contrib/postgres/validators.py ---- --------------------
ANN Missing type annotation for function argument 'a'
class RangeMinValueValidator(MinValueValidator):
def compare(self, a, b): # <--------------------------------!!!
return a.lower is None or a.lower < b
-------------------- samples/libs/django/contrib/admin/utils.py ---- --------------------
ANN Missing type annotation for function argument 'obj'
def model_format_dict(obj): # <--------------------------------!!!
"""
Return a `dict` with keys 'verbose_name' and 'verbose_name_plural',
typically for use with string formatting.
`obj` may be a `Model` instance, `Model` subclass, or `QuerySet` instance.
Во-вторых, джанго любит повыносить мозг:
-------------------- samples/libs/django/utils/translation/__init__.py ---- --------------------
CCR001 Cognitive complexity is too high (17 > 7)
def lazy_number(func, resultclass, number=None, **kwargs): # <--------------------------------!!!
if isinstance(number, int):
kwargs["number"] = number
proxy = lazy(func, resultclass)(**kwargs)
else:
original_kwargs = kwargs.copy()
class NumberAwareString(resultclass):
def __bool__(self):
return bool(kwargs["singular"])
def _get_number_value(self, values):
try:
return values[number]
except KeyError:
raise KeyError(
"Your dictionary lacks key '%s'. Please provide "
"it, because it is required to determine whether "
# ...
-------------------- samples/libs/django/db/models/base.py ---- --------------------
CCR001 Cognitive complexity is too high (67 > 7)
class Model(metaclass=ModelBase):
def __init__(self, *args, **kwargs): # <--------------------------------!!!
# Alias some things as locals to avoid repeat global lookups
cls = self.__class__
opts = self._meta
_setattr = setattr
_DEFERRED = DEFERRED
if opts.abstract:
raise TypeError("Abstract models cannot be instantiated.")
pre_init.send(sender=cls, args=args, kwargs=kwargs)
# Set up the storage for instance state
self._state = ModelState()
# There is a rather weird disparity here; if kwargs, it's set, then args
# overrides it. It should be one or the other; don't duplicate the work
# The reason for the kwargs check is that standard iterator passes in by
# args, and instantiation for iteration is 33% faster.
if len(args) > len(opts.concrete_fields):
# Daft, but matches old exception sans the err msg.
raise IndexError("Number of args exceeds number of fields")
if not kwargs:
fields_iter = iter(opts.concrete_fields)
# The ordering of the zip calls matter - zip throws StopIteration
# when an iter throws it. So if the first iter throws it, the second
# is *not* consumed. We rely on this, so don't change the order
# without changing the logic.
for val, field in zip(args, fields_iter):
if val is _DEFERRED:
continue
_setattr(self, field.attname, val)
else:
# Slower, kwargs-ready version.
# ...
Всякие ненужные if-else - тоже есть в наличии, можете брать оптом.
-------------------- samples/libs/django/contrib/auth/views.py ---- --------------------
GRG002 Missing early quit
class LoginView(RedirectURLMixin, FormView):
# ...
def get_default_redirect_url(self):
"""Return the default redirect URL."""
if self.next_page: # <--------------------------------!!!
return resolve_url(self.next_page)
else:
return resolve_url(settings.LOGIN_REDIRECT_URL)
GRG002 Missing early quit
def distinct_sql(self, fields, params):
"""
Return an SQL DISTINCT clause which removes duplicate rows from the
result set. If any fields are given, only check the given fields for
duplicates.
"""
if fields: # <--------------------------------!!!
raise NotSupportedError(
"DISTINCT ON fields is not supported by this database backend"
)
else:
return ["DISTINCT"], []
-------------------- samples/libs/django/utils/cache.py ---- --------------------
GRG002 Missing early quit
def learn_cache_key(request, response, cache_timeout=None, key_prefix=None, cache=None):
"""
Learn what headers to take into account for some request URL from the
response object. Store those headers in a global URL registry so that
later access to that URL will know what headers to take into account
without building the response object itself. The headers are named in the
Vary header of the response, but we want to prevent response generation.
The list of headers to use for cache key generation is stored in the same
cache as the pages themselves. If the cache ages some data out of the
cache, this just means that we have to build the response once to get at
the Vary header and so at the list of headers to use for the cache key.
"""
if key_prefix is None:
key_prefix = settings.CACHE_MIDDLEWARE_KEY_PREFIX
if cache_timeout is None:
cache_timeout = settings.CACHE_MIDDLEWARE_SECONDS
cache_key = _generate_cache_header_key(key_prefix, request)
if cache is None:
cache = caches[settings.CACHE_MIDDLEWARE_ALIAS]
if response.has_header("Vary"): # <--------------------------------!!!
is_accept_language_redundant = settings.USE_I18N
# If i18n is used, the generated cache key will be suffixed with the
# current locale. Adding the raw value of Accept-Language is redundant
# in that case and would result in storing the same content under
# multiple keys in the cache. See #18191 for details.
headerlist = []
for header in cc_delim_re.split(response.headers["Vary"]):
header = header.upper().replace("-", "_")
if header != "ACCEPT_LANGUAGE" or not is_accept_language_redundant:
headerlist.append("HTTP_" + header)
headerlist.sort()
cache.set(cache_key, headerlist, cache_timeout)
return _generate_cache_key(request, request.method, headerlist, key_prefix)
else:
# if there is no Vary header, we still need a cache key
# for the request.build_absolute_uri()
cache.set(cache_key, [], cache_timeout)
return _generate_cache_key(request, request.method, [], key_prefix)
Ну и любимая нами копипаста, куда ж без неё!
-------------------- samples/libs/django/db/backends/postgresql/client.py ---- --------------------
GRG001 Copy-paste of code
class DatabaseClient(BaseDatabaseClient):
executable_name = "psql"
@classmethod
def settings_to_cmd_args_env(cls, settings_dict, parameters):
args = [cls.executable_name]
options = settings_dict.get("OPTIONS", {}) # <--------------------------------!!!
host = settings_dict.get("HOST")
port = settings_dict.get("PORT")
dbname = settings_dict.get("NAME")
user = settings_dict.get("USER")
passwd = settings_dict.get("PASSWORD")
passfile = options.get("passfile")
service = options.get("service")
sslmode = options.get("sslmode")
-------------------- samples/libs/django/contrib/postgres/apps.py ---- --------------------
GRG001 Copy-paste of code
if (
not enter
and setting == "INSTALLED_APPS"
and "django.contrib.postgres" not in set(value)
):
connection_created.disconnect(register_type_handlers) # <--------------------------------!!!
CharField._unregister_lookup(Unaccent)
TextField._unregister_lookup(Unaccent)
CharField._unregister_lookup(SearchLookup)
TextField._unregister_lookup(SearchLookup)
CharField._unregister_lookup(TrigramSimilar)
TextField._unregister_lookup(TrigramSimilar)
CharField._unregister_lookup(TrigramWordSimilar)
TextField._unregister_lookup(TrigramWordSimilar)
# Disconnect this receiver until the next time this app is installed
-------------------- samples/libs/django/template/smartif.py ---- --------------------
GRG001 Copy-paste of code
"and": infix(7, lambda context, x, y: x.eval(context) and y.eval(context)),
"not": prefix(8, lambda context, x: not x.eval(context)),
"in": infix(9, lambda context, x, y: x.eval(context) in y.eval(context)),
"not in": infix(9, lambda context, x, y: x.eval(context) not in y.eval(context)),
"is": infix(10, lambda context, x, y: x.eval(context) is y.eval(context)),
"is not": infix(10, lambda context, x, y: x.eval(context) is not y.eval(context)),
"==": infix(10, lambda context, x, y: x.eval(context) == y.eval(context)),
"!=": infix(10, lambda context, x, y: x.eval(context) != y.eval(context)), # <--------------------------------!!!
">": infix(10, lambda context, x, y: x.eval(context) > y.eval(context)),
">=": infix(10, lambda context, x, y: x.eval(context) >= y.eval(context)),
"<": infix(10, lambda context, x, y: x.eval(context) < y.eval(context)),
"<=": infix(10, lambda context, x, y: x.eval(context) <= y.eval(context)),
-------------------- samples/libs/django/contrib/gis/geos/io.py ---- --------------------
GRG001 Copy-paste of code
from django.contrib.gis.geos.geometry import GEOSGeometry
from django.contrib.gis.geos.prototypes.io import (
WKBWriter,
WKTWriter,
_WKBReader,
_WKTReader, # <--------------------------------!!!
)
Мы-то теперь с вами знаем про suppress(), а вот джанго - нет ;)
-------------------- samples/libs/django/views/debug.py ---- --------------------
SIM105 Use 'contextlib.suppress(OSError)'
try:
source = loader.get_source(module_name)
except ImportError:
pass
if source is not None:
source = source.splitlines()
if source is None:
try: # <--------------------------------!!!
with open(filename, "rb") as fp:
source = fp.read().splitlines()
except OSError:
pass
return source
fastapi
Ставлю всё на то, что чел, написавший pydantic и fastapi, знает дело и пишет сразу годноту!
Так и есть! Код классный, в основном ошибки - не ошибки, а false positives, лишь иногда встречается всякое:
-------------------- samples/libs/fastapi/dependencies/models.py ---- --------------------
CCR001 Cognitive complexity is too high (8 > 7)
class Dependant:
def __init__( # <--------------------------------!!!
self,
*,
path_params: Optional[List[ModelField]] = None,
query_params: Optional[List[ModelField]] = None,
header_params: Optional[List[ModelField]] = None,
cookie_params: Optional[List[ModelField]] = None,
body_params: Optional[List[ModelField]] = None,
dependencies: Optional[List["Dependant"]] = None,
security_schemes: Optional[List[SecurityRequirement]] = None,
name: Optional[str] = None,
call: Optional[Callable[..., Any]] = None,
request_param_name: Optional[str] = None,
websocket_param_name: Optional[str] = None,
http_connection_param_name: Optional[str] = None,
response_param_name: Optional[str] = None,
background_tasks_param_name: Optional[str] = None,
security_scopes_param_name: Optional[str] = None,
security_scopes: Optional[List[str]] = None,
use_cache: bool = True,
path: Optional[str] = None,
) -> None:
self.path_params = path_params or []
self.query_params = query_params or []
self.header_params = header_params or []
self.cookie_params = cookie_params or []
self.body_params = body_params or []
self.dependencies = dependencies or []
self.security_requirements = security_schemes or []
self.request_param_name = request_param_name
self.websocket_param_name = websocket_param_name
self.http_connection_param_name = http_connection_param_name
self.response_param_name = response_param_name
self.background_tasks_param_name = background_tasks_param_name
mypy
mypy больше всего любит взрывать мозг. Простите за пример:
-------------------- samples/libs/mypy/exprtotype.py ---- --------------------
CCR001 Cognitive complexity is too high (62 > 7)
def expr_to_unanalyzed_type(expr: Expression,
options: Optional[Options] = None,
allow_new_syntax: bool = False,
_parent: Optional[Expression] = None) -> ProperType:
"""Translate an expression to the corresponding type.
The result is not semantically analyzed. It can be UnboundType or TypeList.
Raise TypeTranslationError if the expression cannot represent a type.
If allow_new_syntax is True, allow all type syntax independent of the target
Python version (used in stubs).
"""
# The `parent` parameter is used in recursive calls to provide context for
# understanding whether an CallableArgument is ok.
name: Optional[str] = None
if isinstance(expr, NameExpr):
name = expr.name
if name == 'True':
return RawExpressionType(True, 'builtins.bool', line=expr.line, column=expr.column)
elif name == 'False':
return RawExpressionType(False, 'builtins.bool', line=expr.line, column=expr.column)
else:
return UnboundType(name, line=expr.line, column=expr.column)
elif isinstance(expr, MemberExpr):
fullname = get_member_expr_fullname(expr)
if fullname:
return UnboundType(fullname, line=expr.line, column=expr.column)
else:
raise TypeTranslationError()
elif isinstance(expr, IndexExpr):
base = expr_to_unanalyzed_type(expr.base, options, allow_new_syntax, expr)
if isinstance(base, UnboundType):
if base.args:
raise TypeTranslationError()
if isinstance(expr.index, TupleExpr):
args = expr.index.items
else:
args = [expr.index]
if isinstance(expr.base, RefExpr) and expr.base.fullname in ANNOTATED_TYPE_NAMES:
# TODO: this is not the optimal solution as we are basically getting rid
# of the Annotation definition and only returning the type information,
# losing all the annotations.
return expr_to_unanalyzed_type(args[0], options, allow_new_syntax, expr)
else:
base.args = tuple(expr_to_unanalyzed_type(arg, options, allow_new_syntax, expr)
for arg in args)
if not base.args:
base.empty_tuple_index = True
return base
else:
raise TypeTranslationError()
elif (isinstance(expr, OpExpr)
and expr.op == '|'
and ((options and options.python_version >= (3, 10)) or allow_new_syntax)):
return UnionType([expr_to_unanalyzed_type(expr.left, options, allow_new_syntax),
expr_to_unanalyzed_type(expr.right, options, allow_new_syntax)])
elif isinstance(expr, CallExpr) and isinstance(_parent, ListExpr):
c = expr.callee
names = []
# Go through the dotted member expr chain to get the full arg
# constructor name to look up
while True:
if isinstance(c, NameExpr):
names.append(c.name)
break
elif isinstance(c, MemberExpr):
names.append(c.name)
c = c.expr
else:
raise TypeTranslationError()
arg_const = '.'.join(reversed(names))
# Go through the constructor args to get its name and type.
name = None
default_type = AnyType(TypeOfAny.unannotated)
typ: Type = default_type
for i, arg in enumerate(expr.args):
if expr.arg_names[i] is not None:
if expr.arg_names[i] == "name":
if name is not None:
# Two names
raise TypeTranslationError()
name = _extract_argument_name(arg)
continue
elif expr.arg_names[i] == "type":
if typ is not default_type:
# Two types
raise TypeTranslationError()
typ = expr_to_unanalyzed_type(arg, options, allow_new_syntax, expr)
continue
else:
raise TypeTranslationError()
elif i == 0:
typ = expr_to_unanalyzed_type(arg, options, allow_new_syntax, expr)
elif i == 1:
name = _extract_argument_name(arg)
else:
raise TypeTranslationError()
return CallableArgument(typ, name, arg_const, expr.line, expr.column)
elif isinstance(expr, ListExpr):
return TypeList([expr_to_unanalyzed_type(t, options, allow_new_syntax, expr)
for t in expr.items],
line=expr.line, column=expr.column)
elif isinstance(expr, StrExpr):
return parse_type_string(expr.value, 'builtins.str', expr.line, expr.column,
assume_str_is_unicode=expr.from_python_3)
elif isinstance(expr, BytesExpr):
return parse_type_string(expr.value, 'builtins.bytes', expr.line, expr.column,
assume_str_is_unicode=False)
elif isinstance(expr, UnicodeExpr):
return parse_type_string(expr.value, 'builtins.unicode', expr.line, expr.column,
assume_str_is_unicode=True)
elif isinstance(expr, UnaryExpr):
typ = expr_to_unanalyzed_type(expr.expr, options, allow_new_syntax)
if isinstance(typ, RawExpressionType):
if isinstance(typ.literal_value, int) and expr.op == '-':
typ.literal_value *= -1
return typ
raise TypeTranslationError()
elif isinstance(expr, IntExpr):
return RawExpressionType(expr.value, 'builtins.int', line=expr.line, column=expr.column)
elif isinstance(expr, FloatExpr):
# Floats are not valid parameters for RawExpressionType , so we just
# pass in 'None' for now. We'll report the appropriate error at a later stage.
return RawExpressionType(None, 'builtins.float', line=expr.line, column=expr.column)
elif isinstance(expr, ComplexExpr):
# Same thing as above with complex numbers.
return RawExpressionType(None, 'builtins.complex', line=expr.line, column=expr.column)
elif isinstance(expr, EllipsisExpr):
return EllipsisType(expr.line)
else:
raise TypeTranslationError()
flake8
flake8 оказался очень хорош. Я хотел поглумиться, типа "написали линтер, а сами пишут кое-как", но оказалось ровно наоборот.
-------------------- samples/libs/flake8/utils.py ---- --------------------
CCR001 Cognitive complexity is too high (23 > 7)
def parse_files_to_codes_mapping( # noqa: C901 # <--------------------------------!!!
value_: Union[Sequence[str], str]
) -> List[Tuple[str, List[str]]]:
"""Parse a files-to-codes mapping.
A files-to-codes mapping a sequence of values specified as
`filenames list:codes list ...`. Each of the lists may be separated by
either comma or whitespace tokens.
:param value: String to be parsed and normalized.
"""
if not isinstance(value_, str):
value = "\n".join(value_)
else:
value = value_
ret: List[Tuple[str, List[str]]] = []
if not value.strip():
return ret
class State:
seen_sep = True
seen_colon = False
filenames: List[str] = []
codes: List[str] = []
def _reset() -> None:
if State.codes:
for filename in State.filenames:
ret.append((filename, State.codes))
State.seen_sep = True
State.seen_colon = False
State.filenames = []
State.codes = []
requests
Почти хорошо. Не любит type hints, в редких случаях выносит мозг.
scrapy
О, у меня большие подозрения насчёт scrapy. Мне вообще не нравится их подход к скрейпингу, и мне кажется, что там уж точно что-то нечисто!
Прав ли я? Конечно, нет! scrapy, как и многие, не уважает type hints и иногда мудрит, но в целом терпим.
pytest
Люблю pytest.
Но он почему-то иногда рыбачит:
PIE786 Use precise exception handlers.
def _should_repr_global_name(obj: object) -> bool:
if callable(obj):
return False
try:
return not hasattr(obj, "__name__")
except Exception: # <--------------------------------!!!
return True
И любит лесенки:
-------------------- samples/libs/_pytest/recwarn.py ---- --------------------
CCR001 Cognitive complexity is too high (25 > 7)
@final
class WarningsChecker(WarningsRecorder):
# ...
def __exit__( # <--------------------------------!!!
self,
exc_type: Optional[Type[BaseException]],
exc_val: Optional[BaseException],
exc_tb: Optional[TracebackType],
) -> None:
super().__exit__(exc_type, exc_val, exc_tb)
__tracebackhide__ = True
# only check if we're not currently handling an exception
if exc_type is None and exc_val is None and exc_tb is None:
if self.expected_warning is not None:
if not any(issubclass(r.category, self.expected_warning) for r in self):
__tracebackhide__ = True
fail(
"DID NOT WARN. No warnings of type {} were emitted. "
"The list of emitted warnings is: {}.".format(
self.expected_warning, [each.message for each in self]
)
)
elif self.match_expr is not None:
for r in self:
if issubclass(r.category, self.expected_warning):
if re.compile(self.match_expr).search(str(r.message)):
break
else:
fail(
"DID NOT WARN. No warnings of type {} matching"
" ('{}') were emitted. The list of emitted warnings"
" is: {}.".format(
self.expected_warning,
self.match_expr,
[each.message for each in self],
# ...
sentry-sdk
Вы уже видите тенденцию? Везде проблемы с аннотациями типов и излишней сложностью:
boto3
По моему опыту, амазон не умеет делать что-то просто. Давайте их разнесём!..
Не аннотировано почти ничего, но всё супер-просто. Вынужден признать, это всё-таки очень неплохой результат.
pandas
Ну что, датасаентисты? У меня даже flake прилично задумался, когда анализировал pandas, и я приготовился к худшему.
Я протестировал все остальные библиотеки, пока готовился pandas, и вот что я увидел:
А ничего! К отсутствию типов мы привыкли, выносом мозга нас не удивишь:
-------------------- samples/libs/pandas/core/window/common.py ---- --------------------
CCR001 Cognitive complexity is too high (70 > 7)
def flex_binary_moment(arg1, arg2, f, pairwise=False): # <--------------------------------!!!
if isinstance(arg1, ABCSeries) and isinstance(arg2, ABCSeries):
X, Y = prep_binary(arg1, arg2)
return f(X, Y)
elif isinstance(arg1, ABCDataFrame):
from pandas import DataFrame
def dataframe_from_int_dict(data, frame_template):
result = DataFrame(data, index=frame_template.index)
if len(result.columns) > 0:
result.columns = frame_template.columns[result.columns]
return result
results = {}
if isinstance(arg2, ABCDataFrame):
if pairwise is False:
if arg1 is arg2:
# special case in order to handle duplicate column names
for i in range(len(arg1.columns)):
results[i] = f(arg1.iloc[:, i], arg2.iloc[:, i])
return dataframe_from_int_dict(results, arg1)
else:
if not arg1.columns.is_unique:
raise ValueError("'arg1' columns are not unique")
if not arg2.columns.is_unique:
raise ValueError("'arg2' columns are not unique")
X, Y = arg1.align(arg2, join="outer")
X, Y = prep_binary(X, Y)
res_columns = arg1.columns.union(arg2.columns)
for col in res_columns:
if col in X and col in Y:
results[col] = f(X[col], Y[col])
return DataFrame(results, index=X.index, columns=res_columns)
elif pairwise is True:
results = defaultdict(dict)
for i in range(len(arg1.columns)):
for j in range(len(arg2.columns)):
if j < i and arg2 is arg1:
# Symmetric case
results[i][j] = results[j][i]
else:
results[i][j] = f(
*prep_binary(arg1.iloc[:, i], arg2.iloc[:, j])
)
from pandas import concat
result_index = arg1.index.union(arg2.index)
if len(result_index):
# construct result frame
result = concat(
[
concat(
[results[i][j] for j in range(len(arg2.columns))],
ignore_index=True,
)
for i in range(len(arg1.columns))
],
ignore_index=True,
axis=1,
)
result.columns = arg1.columns
Конец
В статье могут быть неточности. Я не проверял все 2к статей с 60к ошибок и все исходники библиотек. Но я часто смотрел живые примеры ошибок. И вот что я в итоге понял.
type hints никто не любит
все склонны переусложнять код
А теперь plot twist: почти все примеры ошибок я брал из статей с самым высоким рейтингом. Какой из этого вывод? Его озвучил @economist75:
Статья понравилась. Не кодом, который почти что ужасен, а темой макросов в графических пакетах.
Поэтому, в общем-то, не важно, как вы пишете, важно, что вы пишете. Но если вы будете при этом писать чистый, легко читаемый код - это будет вишенка на торте.
Моя повозка: Блог погромиста.
